| Message ID | 1468830508-15244-1-git-send-email-simhavcs@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18 July 2016 at 09:28, Vinay Simha BN <simhavcs@gmail.com> wrote: > Provide a small convenience wrapper that set/get the > display brightness value > > Cc: John Stultz <john.stultz@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Archit Taneja <archit.taneja@gmail.com> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Vinay Simha BN <simhavcs@gmail.com> > > --- > v1: > *tested in nexus7 2nd gen. > > v2: > * implemented jani review comments > -functions name mapped accordingly > -bl value increased from 0xff to 0xffff > -backlight interface will be handled in panel driver, > so it is moved from the mipi_dsi helper function > > v3: > * emil review comments > (err < 0) supposed to be (err <= 0) > --- > drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 4 ++++ > 2 files changed, 53 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index af0d471..43aa743 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) > } > EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > > +/** > + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value > + * of the display > + * @dsi: DSI peripheral device > + * @brightness: brightness value > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, > + u16 *brightness) > +{ > + ssize_t err; > + > + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, > + brightness, sizeof(*brightness)); > + if (err <= 0) { > + if (err == 0) > + err = -ENODATA; > + Looks better now. Thank you. > + return err; Side note: When sizeof(ssize_t) != sizeof(int) this might lead to some very annoying bugs. In practise I doubt anyone uses error codes in the SSIZE_MIN to INT_MIN range so we should be safe. Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Regards, Emil
On Mon, Jul 18, 2016 at 4:28 AM, Vinay Simha BN <simhavcs@gmail.com> wrote: > Provide a small convenience wrapper that set/get the > display brightness value > > Cc: John Stultz <john.stultz@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Archit Taneja <archit.taneja@gmail.com> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Vinay Simha BN <simhavcs@gmail.com> > > --- > v1: > *tested in nexus7 2nd gen. > > v2: > * implemented jani review comments > -functions name mapped accordingly > -bl value increased from 0xff to 0xffff > -backlight interface will be handled in panel driver, > so it is moved from the mipi_dsi helper function > > v3: > * emil review comments > (err < 0) supposed to be (err <= 0) > --- > drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 4 ++++ > 2 files changed, 53 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index af0d471..43aa743 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) > } > EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > > +/** > + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value > + * of the display > + * @dsi: DSI peripheral device > + * @brightness: brightness value > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, > + u16 *brightness) > +{ > + ssize_t err; > + > + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, > + brightness, sizeof(*brightness)); > + if (err <= 0) { > + if (err == 0) > + err = -ENODATA; > + > + return err; > + } > + > + return 0; IMO, this is still pretty awkward. I think the following is a bit more conventional: err = mipi_dsi_dcs_... if (err == 0) return -ENODATA; else if (err < 0) return err; return 0; > +} > +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness); > + > +/** > + * mipi_dsi_dcs_set_display_brightness() - sets the brightness value of > + * the display > + * @dsi: DSI peripheral device > + * @brightness: brightness value > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, > + u16 brightness) > +{ > + ssize_t err; > + u8 bl_value[2] = { brightness & 0xff, brightness >> 8 }; > + > + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, > + bl_value, sizeof(bl_value)); > + if (err < 0) > + return err; > + > + return 0; > +} > +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness); > + > static int mipi_dsi_drv_probe(struct device *dev) > { > struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index 47ac925..404c373 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -270,6 +270,10 @@ int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); > int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, > enum mipi_dsi_dcs_tear_mode mode); > int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); > +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, > + u16 *brightness); > +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, > + u16 brightness); > > /** > * struct mipi_dsi_driver - DSI driver > -- > 2.1.2 >
just to be inline with the existing code (funcs: mipi_dsi_dcs_get_power_mode, mipi_dsi_dcs_get_pixel_format) in drivers/gpu/drm/drm_mipi_dsi.c followed the same for mipi_dsi_dcs_get_display_brightness We may need to change as you suggested for other two functions also. please suggest. On Tue, Jul 19, 2016 at 3:00 PM, Sean Paul <seanpaul@google.com> wrote: > On Mon, Jul 18, 2016 at 4:28 AM, Vinay Simha BN <simhavcs@gmail.com> wrote: >> Provide a small convenience wrapper that set/get the >> display brightness value >> >> Cc: John Stultz <john.stultz@linaro.org> >> Cc: Sumit Semwal <sumit.semwal@linaro.org> >> Cc: Archit Taneja <archit.taneja@gmail.com> >> Cc: Rob Clark <robdclark@gmail.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Cc: Emil Velikov <emil.l.velikov@gmail.com> >> Signed-off-by: Vinay Simha BN <simhavcs@gmail.com> >> >> --- >> v1: >> *tested in nexus7 2nd gen. >> >> v2: >> * implemented jani review comments >> -functions name mapped accordingly >> -bl value increased from 0xff to 0xffff >> -backlight interface will be handled in panel driver, >> so it is moved from the mipi_dsi helper function >> >> v3: >> * emil review comments >> (err < 0) supposed to be (err <= 0) >> --- >> drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ >> include/drm/drm_mipi_dsi.h | 4 ++++ >> 2 files changed, 53 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c >> index af0d471..43aa743 100644 >> --- a/drivers/gpu/drm/drm_mipi_dsi.c >> +++ b/drivers/gpu/drm/drm_mipi_dsi.c >> @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) >> } >> EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); >> >> +/** >> + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value >> + * of the display >> + * @dsi: DSI peripheral device >> + * @brightness: brightness value >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, >> + u16 *brightness) >> +{ >> + ssize_t err; >> + >> + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, >> + brightness, sizeof(*brightness)); >> + if (err <= 0) { >> + if (err == 0) >> + err = -ENODATA; >> + >> + return err; >> + } >> + >> + return 0; > > IMO, this is still pretty awkward. I think the following is a bit more > conventional: > > err = mipi_dsi_dcs_... > if (err == 0) > return -ENODATA; > else if (err < 0) > return err; > > return 0; > >> +} >> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness); >> + >> +/** >> + * mipi_dsi_dcs_set_display_brightness() - sets the brightness value of >> + * the display >> + * @dsi: DSI peripheral device >> + * @brightness: brightness value >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, >> + u16 brightness) >> +{ >> + ssize_t err; >> + u8 bl_value[2] = { brightness & 0xff, brightness >> 8 }; >> + >> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, >> + bl_value, sizeof(bl_value)); >> + if (err < 0) >> + return err; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness); >> + >> static int mipi_dsi_drv_probe(struct device *dev) >> { >> struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); >> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h >> index 47ac925..404c373 100644 >> --- a/include/drm/drm_mipi_dsi.h >> +++ b/include/drm/drm_mipi_dsi.h >> @@ -270,6 +270,10 @@ int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); >> int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, >> enum mipi_dsi_dcs_tear_mode mode); >> int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); >> +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, >> + u16 *brightness); >> +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, >> + u16 brightness); >> >> /** >> * struct mipi_dsi_driver - DSI driver >> -- >> 2.1.2 >>
On Tue, Jul 19, 2016 at 5:42 AM, Vinay Simha <simhavcs@gmail.com> wrote: > just to be inline with the existing code (funcs: > mipi_dsi_dcs_get_power_mode, mipi_dsi_dcs_get_pixel_format) in > drivers/gpu/drm/drm_mipi_dsi.c followed the same for > mipi_dsi_dcs_get_display_brightness > > We may need to change as you suggested for other two functions also. > please suggest. > Ah, didn't see that. I don't really care either way, tbh, just something that struck me as odd. Feel free to ignore. Sean > > > > On Tue, Jul 19, 2016 at 3:00 PM, Sean Paul <seanpaul@google.com> wrote: >> On Mon, Jul 18, 2016 at 4:28 AM, Vinay Simha BN <simhavcs@gmail.com> wrote: >>> Provide a small convenience wrapper that set/get the >>> display brightness value >>> >>> Cc: John Stultz <john.stultz@linaro.org> >>> Cc: Sumit Semwal <sumit.semwal@linaro.org> >>> Cc: Archit Taneja <archit.taneja@gmail.com> >>> Cc: Rob Clark <robdclark@gmail.com> >>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>> Cc: Thierry Reding <thierry.reding@gmail.com> >>> Cc: Emil Velikov <emil.l.velikov@gmail.com> >>> Signed-off-by: Vinay Simha BN <simhavcs@gmail.com> >>> >>> --- >>> v1: >>> *tested in nexus7 2nd gen. >>> >>> v2: >>> * implemented jani review comments >>> -functions name mapped accordingly >>> -bl value increased from 0xff to 0xffff >>> -backlight interface will be handled in panel driver, >>> so it is moved from the mipi_dsi helper function >>> >>> v3: >>> * emil review comments >>> (err < 0) supposed to be (err <= 0) >>> --- >>> drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ >>> include/drm/drm_mipi_dsi.h | 4 ++++ >>> 2 files changed, 53 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c >>> index af0d471..43aa743 100644 >>> --- a/drivers/gpu/drm/drm_mipi_dsi.c >>> +++ b/drivers/gpu/drm/drm_mipi_dsi.c >>> @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) >>> } >>> EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); >>> >>> +/** >>> + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value >>> + * of the display >>> + * @dsi: DSI peripheral device >>> + * @brightness: brightness value >>> + * >>> + * Return: 0 on success or a negative error code on failure. >>> + */ >>> +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, >>> + u16 *brightness) >>> +{ >>> + ssize_t err; >>> + >>> + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, >>> + brightness, sizeof(*brightness)); >>> + if (err <= 0) { >>> + if (err == 0) >>> + err = -ENODATA; >>> + >>> + return err; >>> + } >>> + >>> + return 0; >> >> IMO, this is still pretty awkward. I think the following is a bit more >> conventional: >> >> err = mipi_dsi_dcs_... >> if (err == 0) >> return -ENODATA; >> else if (err < 0) >> return err; >> >> return 0; >> >>> +} >>> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness); >>> + >>> +/** >>> + * mipi_dsi_dcs_set_display_brightness() - sets the brightness value of >>> + * the display >>> + * @dsi: DSI peripheral device >>> + * @brightness: brightness value >>> + * >>> + * Return: 0 on success or a negative error code on failure. >>> + */ >>> +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, >>> + u16 brightness) >>> +{ >>> + ssize_t err; >>> + u8 bl_value[2] = { brightness & 0xff, brightness >> 8 }; >>> + >>> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, >>> + bl_value, sizeof(bl_value)); >>> + if (err < 0) >>> + return err; >>> + >>> + return 0; >>> +} >>> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness); >>> + >>> static int mipi_dsi_drv_probe(struct device *dev) >>> { >>> struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); >>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h >>> index 47ac925..404c373 100644 >>> --- a/include/drm/drm_mipi_dsi.h >>> +++ b/include/drm/drm_mipi_dsi.h >>> @@ -270,6 +270,10 @@ int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); >>> int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, >>> enum mipi_dsi_dcs_tear_mode mode); >>> int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); >>> +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, >>> + u16 *brightness); >>> +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, >>> + u16 brightness); >>> >>> /** >>> * struct mipi_dsi_driver - DSI driver >>> -- >>> 2.1.2 >>> > > > > -- > Regards, > > Vinay Simha.B.N.
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index af0d471..43aa743 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) } EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); +/** + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value + * of the display + * @dsi: DSI peripheral device + * @brightness: brightness value + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, + u16 *brightness) +{ + ssize_t err; + + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, + brightness, sizeof(*brightness)); + if (err <= 0) { + if (err == 0) + err = -ENODATA; + + return err; + } + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness); + +/** + * mipi_dsi_dcs_set_display_brightness() - sets the brightness value of + * the display + * @dsi: DSI peripheral device + * @brightness: brightness value + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, + u16 brightness) +{ + ssize_t err; + u8 bl_value[2] = { brightness & 0xff, brightness >> 8 }; + + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, + bl_value, sizeof(bl_value)); + if (err < 0) + return err; + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness); + static int mipi_dsi_drv_probe(struct device *dev) { struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 47ac925..404c373 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -270,6 +270,10 @@ int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, enum mipi_dsi_dcs_tear_mode mode); int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, + u16 *brightness); +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, + u16 brightness); /** * struct mipi_dsi_driver - DSI driver
Provide a small convenience wrapper that set/get the display brightness value Cc: John Stultz <john.stultz@linaro.org> Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: Archit Taneja <archit.taneja@gmail.com> Cc: Rob Clark <robdclark@gmail.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Vinay Simha BN <simhavcs@gmail.com> --- v1: *tested in nexus7 2nd gen. v2: * implemented jani review comments -functions name mapped accordingly -bl value increased from 0xff to 0xffff -backlight interface will be handled in panel driver, so it is moved from the mipi_dsi helper function v3: * emil review comments (err < 0) supposed to be (err <= 0) --- drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 4 ++++ 2 files changed, 53 insertions(+)