Received-SPF: pass (google.com: domain of linux-kernel+bounces-160321-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240424172017.1.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid>
 <87pludq2g0.fsf@intel.com> <CAD=FV=W+Pcr+voBkcfeE_UC+ukN_hLXgoqMk0watROWRXe_2dg@mail.gmail.com>
 <beqsovvdkvn63prt3c6b3epb6tachff35vpaf62dfkwof7kwht@u3p7bkv7owro> <CAD=FV=WxYoFYefdZ4PQ=QF5aHpeWoC3qM1b5d2vf_qBH90ZMQw@mail.gmail.com>
In-Reply-To: <CAD=FV=WxYoFYefdZ4PQ=QF5aHpeWoC3qM1b5d2vf_qBH90ZMQw@mail.gmail.com>
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Fri, 26 Apr 2024 18:42:25 +0300
Message-ID: <CAA8EJpodW+VjUzBh1VLC2OeUaMhoX7FgNrsNStq5rPbOnyp95g@mail.gmail.com>
Subject: Re: [PATCH] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
To: Doug Anderson <dianders@chromium.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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, 26 Apr 2024 at 18:41, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Apr 25, 2024 at 8:03=E2=80=AFPM 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=E2=80=AFAM Jani Nikula <jani.nikula@linu=
x.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 l=
en);
> > > > >  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, v=
oid *data,
> > > > > @@ -317,14 +321,10 @@ int mipi_dsi_dcs_get_display_brightness_lar=
ge(struct mipi_dsi_device *dsi,
> > > > >  #define mipi_dsi_generic_write_seq(dsi, seq...)                 =
               \
> > > > >       do {                                                       =
            \
> > > > >               static const u8 d[] =3D { seq };                   =
              \
> > > > > -             struct device *dev =3D &dsi->dev;                  =
              \
> > > > >               int ret;                                           =
            \
> > > > > -             ret =3D mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d=
));           \
> > > > > -             if (ret < 0) {                                     =
            \
> > > > > -                     dev_err_ratelimited(dev, "transmit data fai=
led: %d\n", \
> > > > > -                                         ret);                  =
            \
> > > > > +             ret =3D 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[] =3D { 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...

Yes, I'm fine with that.

--=20
With best wishes
Dmitry