| Message ID | 20201124124538.660710-69-tomi.valkeinen@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
Hi Tomi, On Tue, Nov 24, 2020 at 02:45:26PM +0200, Tomi Valkeinen wrote: > We have a useless 'if' in the dsicm_bl_update_status(), a left over from > the conversion to DRM model. Drop the if. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> But with a few comments. > --- > drivers/gpu/drm/panel/panel-dsi-cm.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 1e7f73340736..c17ed728c695 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -194,7 +194,7 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata) > static int dsicm_bl_update_status(struct backlight_device *dev) > { > struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); > - int r = 0; > + int r; > int level; > > if (dev->props.fb_blank == FB_BLANK_UNBLANK && Unrelated to this patch - but here it would be better to use level = backlight_get_brightness(dev); level is 0 is backlight is off. Sam > @@ -207,11 +207,9 @@ static int dsicm_bl_update_status(struct backlight_device *dev) > > mutex_lock(&ddata->lock); > > - if (ddata->enabled) { > - if (!r) > - r = dsicm_dcs_write_1( > - ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level); > - } > + if (ddata->enabled) > + r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, > + level); > > mutex_unlock(&ddata->lock); > > -- > 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 01/12/2020 02:22, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tue, Nov 24, 2020 at 02:45:26PM +0200, Tomi Valkeinen wrote: >> We have a useless 'if' in the dsicm_bl_update_status(), a left over from >> the conversion to DRM model. Drop the if. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/panel/panel-dsi-cm.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c >> index 1e7f73340736..c17ed728c695 100644 >> --- a/drivers/gpu/drm/panel/panel-dsi-cm.c >> +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c >> @@ -194,7 +194,7 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata) >> static int dsicm_bl_update_status(struct backlight_device *dev) >> { >> struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); >> - int r = 0; >> + int r; >> int level; >> >> if (dev->props.fb_blank == FB_BLANK_UNBLANK && >> @@ -207,11 +207,9 @@ static int dsicm_bl_update_status(struct backlight_device *dev) >> >> mutex_lock(&ddata->lock); >> >> - if (ddata->enabled) { >> - if (!r) >> - r = dsicm_dcs_write_1( >> - ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level); >> - } >> + if (ddata->enabled) >> + r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, >> + level); > > If !ddata->enabled, won't r be uninitialized ? Yes. I wonder how I missed that... I'll keep the "r = 0" initialization. Tomi
diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c index 1e7f73340736..c17ed728c695 100644 --- a/drivers/gpu/drm/panel/panel-dsi-cm.c +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c @@ -194,7 +194,7 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata) static int dsicm_bl_update_status(struct backlight_device *dev) { struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev); - int r = 0; + int r; int level; if (dev->props.fb_blank == FB_BLANK_UNBLANK && @@ -207,11 +207,9 @@ static int dsicm_bl_update_status(struct backlight_device *dev) mutex_lock(&ddata->lock); - if (ddata->enabled) { - if (!r) - r = dsicm_dcs_write_1( - ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level); - } + if (ddata->enabled) + r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, + level); mutex_unlock(&ddata->lock);
We have a useless 'if' in the dsicm_bl_update_status(), a left over from the conversion to DRM model. Drop the if. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/panel/panel-dsi-cm.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)