| Message ID | 20210608143159.1.I230026301243fbcee23d408c75aa468c1fec58f7@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: Fix error code on failure to set brightness | expand |
On 2021-06-08 12:32 a.m., Anand K Mistry wrote: > The backlight_ops.update_status function is required to return a > negative error code on failure. Returning a positive code may be > interpreted as a success. This is true for the 'brightness' sysfs file, > which passes through a non-zero value as the return value of the write() > syscall. This is interpreted in user-space as a successful write of 1 > character, which is obviously wrong. > > It's not clear exactly what error code to use, but EINVAL should be > reasonable. > > Signed-off-by: Anand K Mistry <amistry@google.com> Thanks for your patch but this code has changed on amd-staging-drm-next. Can you try with the latest? The new backlight_update_status will always return 0 now. Harry > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 652cc1a0e450..ad322613390d 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3431,7 +3431,7 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd) > else > rc = dc_link_set_backlight_level(dm->backlight_link, brightness, 0); > > - return rc ? 0 : 1; > + return rc ? 0 : -EINVAL; > } > > static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd) >
> > Thanks for your patch but this code has changed on amd-staging-drm-next. > > Can you try with the latest? The new backlight_update_status will always > return 0 now. Perfect! This solves my actual issue which is that restoring the backlight level is failing on my device. Thank you.
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 652cc1a0e450..ad322613390d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3431,7 +3431,7 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd) else rc = dc_link_set_backlight_level(dm->backlight_link, brightness, 0); - return rc ? 0 : 1; + return rc ? 0 : -EINVAL; } static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
The backlight_ops.update_status function is required to return a negative error code on failure. Returning a positive code may be interpreted as a success. This is true for the 'brightness' sysfs file, which passes through a non-zero value as the return value of the write() syscall. This is interpreted in user-space as a successful write of 1 character, which is obviously wrong. It's not clear exactly what error code to use, but EINVAL should be reasonable. Signed-off-by: Anand K Mistry <amistry@google.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)