[05/12] drm/sun4i: sun6i_mipi_dsi: Add DSI Generic short write 2 param transfer

Submitted by Jagan Teki on Sept. 27, 2018, 11:48 a.m.

Details

Message ID 20180927114850.24565-6-jagan@amarulasolutions.com
State New
Headers show
Series "drm/sun4i: Allwinner A64 MIPI-DSI support" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Jagan Teki Sept. 27, 2018, 11:48 a.m.
Short transfer write support for DCS and Generic transfer types
share similar way to process command sequence in DSI block so
add generic write 2 param transfer type macro so-that the panels
which are requesting similar transfer type may process properly.

Also added error check for unsupporting transfer types this make
debugging easy for new transfer types.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 156b371243c6..1c7e42015645 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -869,6 +869,7 @@  static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
 		     SUN6I_DSI_CMD_CTL_TX_FLAG);
 
 	switch (msg->type) {
+	case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
 	case MIPI_DSI_DCS_SHORT_WRITE:
 	case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
 		ret = sun6i_dsi_dcs_write_short(dsi, msg);
@@ -885,6 +886,8 @@  static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
 		}
 
 	default:
+		dev_err(dsi->dev, "unsupported message type 0x%02x\n",
+			msg->type);
 		ret = -EINVAL;
 	}
 

Comments

On Thu, Sep 27, 2018 at 10:48 PM Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> On Thu, Sep 27, 2018 at 05:18:43PM +0530, Jagan Teki wrote:
> > Short transfer write support for DCS and Generic transfer types
> > share similar way to process command sequence in DSI block so
> > add generic write 2 param transfer type macro so-that the panels
> > which are requesting similar transfer type may process properly.
> >
> > Also added error check for unsupporting transfer types this make
> > debugging easy for new transfer types.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index 156b371243c6..1c7e42015645 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -869,6 +869,7 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> >                    SUN6I_DSI_CMD_CTL_TX_FLAG);
> >
> >       switch (msg->type) {
> > +     case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
> >       case MIPI_DSI_DCS_SHORT_WRITE:
> >       case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
>
> You should order them by alphabetical order.

I just placed it with transfer code, ok will add it alphabetical

>
> >               ret = sun6i_dsi_dcs_write_short(dsi, msg);
> > @@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> >               }
> >
> >       default:
> > +             dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> > +                     msg->type);
>
> And this isn't an error check.

But unsupported message type by sun6i_dsi should be an error
eventually isn't it? and we can easily figure out where the error
trigger.
On Thu, Sep 27, 2018 at 11:06:50PM +0530, Jagan Teki wrote:
> >
> > >               ret = sun6i_dsi_dcs_write_short(dsi, msg);
> > > @@ -885,6 +886,8 @@ static ssize_t sun6i_dsi_transfer(struct mipi_dsi_host *host,
> > >               }
> > >
> > >       default:
> > > +             dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> > > +                     msg->type);
> >
> > And this isn't an error check.
> 
> But unsupported message type by sun6i_dsi should be an error
> eventually isn't it?

It's already an error condition. What you're adding, and unlike what
your commit log says, is not an error check...

> and we can easily figure out where the error trigger.

... but instead an error message.

That's definitely not the same thing, and I'm not sure we actually
need it. If a driver requests multiple transfers that are unsupported,
we'll end up spaming the kernel logs, especially when it can and
should be checked in the driver doing those transfers.

Maxime