| Message ID | 20201124124538.660710-61-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
On Tue, Nov 24, 2020 at 02:45:18PM +0200, Tomi Valkeinen wrote: > Set the column & page address once during setup, instead of relying the > DSI host driver to set those. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> But one comment below. > --- > drivers/gpu/drm/panel/panel-dsi-cm.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 59e8e6b18e97..1e7f73340736 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -171,6 +171,26 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) > return 0; > } > > +static int dsicm_set_update_window(struct panel_drv_data *ddata) > +{ > + struct mipi_dsi_device *dsi = ddata->dsi; > + int r; > + u16 x1 = 0; > + u16 x2 = ddata->mode.hdisplay - 1; > + u16 y1 = 0; > + u16 y2 = ddata->mode.vdisplay - 1; It does not help the readability with the variables. Using r = mipi_dsi_dcs_set_column_address(dsi, 0, ddata->mode.hdisplay - 1); would IMO be easier to parse. Then the pointless assignment of x1 and y1 are dropped too. But nitpicking, ignore if think the current code is better. Sam > + > + r = mipi_dsi_dcs_set_column_address(dsi, x1, x2); > + if (r < 0) > + return r; > + > + r = mipi_dsi_dcs_set_page_address(dsi, y1, y2); > + if (r < 0) > + return r; > + > + return 0; > +} > + > static int dsicm_bl_update_status(struct backlight_device *dev) > { > struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); > @@ -308,6 +328,10 @@ static int dsicm_power_on(struct panel_drv_data *ddata) > if (r) > goto err; > > + r = dsicm_set_update_window(ddata); > + if (r) > + goto err; > + > r = mipi_dsi_dcs_set_display_on(ddata->dsi); > if (r) > goto err; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 24/11/2020 19:30, Sam Ravnborg wrote: > On Tue, Nov 24, 2020 at 02:45:18PM +0200, Tomi Valkeinen wrote: >> Set the column & page address once during setup, instead of relying the >> DSI host driver to set those. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > But one comment below. > >> --- >> drivers/gpu/drm/panel/panel-dsi-cm.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c >> index 59e8e6b18e97..1e7f73340736 100644 >> --- a/drivers/gpu/drm/panel/panel-dsi-cm.c >> +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c >> @@ -171,6 +171,26 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) >> return 0; >> } >> >> +static int dsicm_set_update_window(struct panel_drv_data *ddata) >> +{ >> + struct mipi_dsi_device *dsi = ddata->dsi; >> + int r; >> + u16 x1 = 0; >> + u16 x2 = ddata->mode.hdisplay - 1; >> + u16 y1 = 0; >> + u16 y2 = ddata->mode.vdisplay - 1; > > It does not help the readability with the variables. > > Using > > r = mipi_dsi_dcs_set_column_address(dsi, 0, ddata->mode.hdisplay - 1); > > would IMO be easier to parse. > > Then the pointless assignment of x1 and y1 are dropped too. I think I agree with you here, although the variables do make it clearer to the reader that the parameters are x1 and x2, not x and w. Tomi
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:18PM +0200, Tomi Valkeinen wrote: > Set the column & page address once during setup, instead of relying the > DSI host driver to set those. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> With Sam's comments addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/panel/panel-dsi-cm.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 59e8e6b18e97..1e7f73340736 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -171,6 +171,26 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) > return 0; > } > > +static int dsicm_set_update_window(struct panel_drv_data *ddata) > +{ > + struct mipi_dsi_device *dsi = ddata->dsi; > + int r; > + u16 x1 = 0; > + u16 x2 = ddata->mode.hdisplay - 1; > + u16 y1 = 0; > + u16 y2 = ddata->mode.vdisplay - 1; > + > + r = mipi_dsi_dcs_set_column_address(dsi, x1, x2); > + if (r < 0) > + return r; > + > + r = mipi_dsi_dcs_set_page_address(dsi, y1, y2); > + if (r < 0) > + return r; > + > + return 0; > +} > + > static int dsicm_bl_update_status(struct backlight_device *dev) > { > struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); > @@ -308,6 +328,10 @@ static int dsicm_power_on(struct panel_drv_data *ddata) > if (r) > goto err; > > + r = dsicm_set_update_window(ddata); > + if (r) > + goto err; > + > r = mipi_dsi_dcs_set_display_on(ddata->dsi); > if (r) > goto err;
Hi, On Tue, Nov 24, 2020 at 02:45:18PM +0200, Tomi Valkeinen wrote: > Set the column & page address once during setup, instead of relying the > DSI host driver to set those. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- I'm fine with or without removal of the variables: Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/gpu/drm/panel/panel-dsi-cm.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 59e8e6b18e97..1e7f73340736 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -171,6 +171,26 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) > return 0; > } > > +static int dsicm_set_update_window(struct panel_drv_data *ddata) > +{ > + struct mipi_dsi_device *dsi = ddata->dsi; > + int r; > + u16 x1 = 0; > + u16 x2 = ddata->mode.hdisplay - 1; > + u16 y1 = 0; > + u16 y2 = ddata->mode.vdisplay - 1; > + > + r = mipi_dsi_dcs_set_column_address(dsi, x1, x2); > + if (r < 0) > + return r; > + > + r = mipi_dsi_dcs_set_page_address(dsi, y1, y2); > + if (r < 0) > + return r; > + > + return 0; > +} > + > static int dsicm_bl_update_status(struct backlight_device *dev) > { > struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); > @@ -308,6 +328,10 @@ static int dsicm_power_on(struct panel_drv_data *ddata) > if (r) > goto err; > > + r = dsicm_set_update_window(ddata); > + if (r) > + goto err; > + > r = mipi_dsi_dcs_set_display_on(ddata->dsi); > if (r) > goto err; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c index 59e8e6b18e97..1e7f73340736 100644 --- a/drivers/gpu/drm/panel/panel-dsi-cm.c +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c @@ -171,6 +171,26 @@ static int dsicm_get_id(struct panel_drv_data *ddata, u8 *id1, u8 *id2, u8 *id3) return 0; } +static int dsicm_set_update_window(struct panel_drv_data *ddata) +{ + struct mipi_dsi_device *dsi = ddata->dsi; + int r; + u16 x1 = 0; + u16 x2 = ddata->mode.hdisplay - 1; + u16 y1 = 0; + u16 y2 = ddata->mode.vdisplay - 1; + + r = mipi_dsi_dcs_set_column_address(dsi, x1, x2); + if (r < 0) + return r; + + r = mipi_dsi_dcs_set_page_address(dsi, y1, y2); + if (r < 0) + return r; + + return 0; +} + static int dsicm_bl_update_status(struct backlight_device *dev) { struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); @@ -308,6 +328,10 @@ static int dsicm_power_on(struct panel_drv_data *ddata) if (r) goto err; + r = dsicm_set_update_window(ddata); + if (r) + goto err; + r = mipi_dsi_dcs_set_display_on(ddata->dsi); if (r) goto err;
Set the column & page address once during setup, instead of relying the DSI host driver to set those. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/panel/panel-dsi-cm.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)