All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	dri-devel@lists.freedesktop.org,
	 Javier Martinez Canillas <javierm@redhat.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	linus.walleij@linaro.org,
	 Cong Yang <yangcong5@huaqin.corp-partner.google.com>,
	 lvzhaoxiong@huaqin.corp-partner.google.com,
	Hsin-Yi Wang <hsinyi@google.com>,
	 Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>,
	David Airlie <airlied@gmail.com>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	 Thomas Zimmermann <tzimmermann@suse.de>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
Date: Fri, 26 Apr 2024 08:35:24 -0700	[thread overview]
Message-ID: <CAD=FV=WxYoFYefdZ4PQ=QF5aHpeWoC3qM1b5d2vf_qBH90ZMQw@mail.gmail.com> (raw)
In-Reply-To: <beqsovvdkvn63prt3c6b3epb6tachff35vpaf62dfkwof7kwht@u3p7bkv7owro>

Hi,

On Thu, Apr 25, 2024 at 8:03 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, Apr 25, 2024 at 10:04:49AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Apr 25, 2024 at 1:19 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >
> > > > @@ -279,6 +281,8 @@ enum mipi_dsi_dcs_tear_mode {
> > > >
> > > >  ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi,
> > > >                                 const void *data, size_t len);
> > > > +ssize_t mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
> > > > +                                      const void *data, size_t len);
> > > >  ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
> > > >                          const void *data, size_t len);
> > > >  ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
> > > > @@ -317,14 +321,10 @@ int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
> > > >  #define mipi_dsi_generic_write_seq(dsi, seq...)                                \
> > > >       do {                                                                   \
> > > >               static const u8 d[] = { seq };                                 \
> > > > -             struct device *dev = &dsi->dev;                                \
> > > >               int ret;                                                       \
> > > > -             ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));           \
> > > > -             if (ret < 0) {                                                 \
> > > > -                     dev_err_ratelimited(dev, "transmit data failed: %d\n", \
> > > > -                                         ret);                              \
> > > > +             ret = mipi_dsi_generic_write_chatty(dsi, d, ARRAY_SIZE(d));    \
> > > > +             if (ret < 0)                                                   \
> > > >                       return ret;                                            \
> > > > -             }                                                              \
> > > >       } while (0)
>
>
> Reading the thread makes me wonder whether we should be going into
> slightly other direction:
>
> Add __must_check() to mipi_dsi_ writing functions,
>
> #define mipi_dsi_dcs_whatever_write(dsi, cmd, seq...)   \
>         ({                                              \
>                 static const u8 d[] = { cmd, seq };     \
>                 mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d));    \
>         })
>
> Then in panel drivers we actually have to explicitly handle the return
> code (either by dropping to the error label or by just returning an
> error).

Given the sheer number of init commands needed by some panels (see
j606f_boe_init_sequence() for instance) I'm still convinced that we
want something that allows people to write their init code in a way
that's not quite so verbose. It sounds as if Jani is OK w/ the
proposal of using the "accumulated return value" (proposal #2 I had).
I'm hoping you're OK w/ that too...

-Doug

  reply	other threads:[~2024-04-26 15:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25  0:20 [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() Douglas Anderson
2024-04-25  7:55 ` Neil Armstrong
2024-04-25  8:19 ` Jani Nikula
2024-04-25 17:04   ` Doug Anderson
2024-04-26  3:03     ` Dmitry Baryshkov
2024-04-26 15:35       ` Doug Anderson [this message]
2024-04-26 15:42         ` Dmitry Baryshkov
2024-04-26 10:08     ` Jani Nikula
2024-04-26 15:28       ` Doug Anderson
2024-04-27  0:01         ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD=FV=WxYoFYefdZ4PQ=QF5aHpeWoC3qM1b5d2vf_qBH90ZMQw@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hsinyi@google.com \
    --cc=jani.nikula@linux.intel.com \
    --cc=javierm@redhat.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lvzhaoxiong@huaqin.corp-partner.google.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=sam@ravnborg.org \
    --cc=tzimmermann@suse.de \
    --cc=yangcong5@huaqin.corp-partner.google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.