Re: [PATCH] drm/amd/display: set backlight level limit to 1
On 2018-10-26 2:44 a.m., Guttula, Suresh wrote:
>
>
> On 10/26/2018 12:04 AM, Wentland, Harry wrote:
>> On 2018-10-25 2:58 a.m., Guttula, Suresh wrote:
>>> This patch will work as workaround for silicon limitation
>>> related to PWM dutycycle when the backlight level goes to 0.
>>>
>>> Actually PWM value is 16 bit value and valid range from 1-65535.
>>> when ever user requested to set this PWM value to 0 which is not
>>> fall in the range, in VBIOS taken care this by limiting to 1.
>>> This patch here will do the same. Either driver or VBIOS can not
>>> pass 0 value as it is not a valid range for PWM and it will
>>> give a high PWM pulse which is not the intented behaviour as
>>> per HW constraints.
>>>
>>
>>
>> Comments from Windows dev, Anthony, CCed here as well:
>>> In Windows, we typically never set backlight PWM value to 0.
>>> Windows brightness slider is from 0 - 100% brightness.
>>> 0% brightness maps to some non-zero value. So user never gets the chance to
>>> set value to 0.
>>>
>>> I'm not 100% sure what kind of flashing they are seeing.
>>> I thought 0 PWM works, and just means PWM signal always low, I think it
>>> would equate to panel backlight completely being off.
>>> I do know of a flashing issue caused by fractional PWM at low PWM values,
>>> but this would not even be at 0. The flashing problem would be anywhere
>>> from 1 - 300 for example. (out of 65535)
>>
>> Do you see flickering when value is at 0 or in the 1 > 0 transition? 1 would
>> be 65535/256 = 256 and put us right in the 1-300 range that has a flashing
>> problem.
>>
>> Harry
>>
>
> We can not make out if flicker is at transition from 1 to 0 or exactly
> at 0, because the flickering is very instantaneous.But, I am not able to
> see the issue @1 , only seen the issue after going down from 1.
>
Apparently Windows driver also checks ACPI ATIF, VBIOS, and the Windows
registry for backlight range information. When no info is present it limits
backlight to the 12-255 range. We might want to implement the ATIF and VBIOS
checks on amdgpu as well, but for now I think your fix is good.
Reviewed-by: Harry Wentland
Harry
> Thanks,
> Suresh G
>
>>> Signed-off-by: suresh guttula
>>> ---
>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
>>> 1 file changed, 8 insertions(+)
>>>
>>> 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 492230c..38f84b2 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -1518,6 +1518,14 @@ static int amdgpu_dm_backlight_update_status(struct
>>> backlight_device *bd)
>>> {
>>> struct amdgpu_display_manager *dm = bl_get_data(bd);
>>>
>>> + /*
>>> +* When we use brightness low key to reduce the brightness,
>>> +* brightness level reaching to 0, with which we can see flash
>>> +* screen on ui beacuse of HW limitation.To avoid that we are
>>> +* limiting level to 1
>>> +*/
>>> + if (bd->props.brightness < 1)
>>> + return 1;
>>> if (dc_link_set_backlight_level(dm->backlight_link,
>>> bd->props.brightness, 0, 0))
>>> return 0;
>>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: set backlight level limit to 1
On 10/26/2018 12:04 AM, Wentland, Harry wrote:
> On 2018-10-25 2:58 a.m., Guttula, Suresh wrote:
>> This patch will work as workaround for silicon limitation
>> related to PWM dutycycle when the backlight level goes to 0.
>>
>> Actually PWM value is 16 bit value and valid range from 1-65535.
>> when ever user requested to set this PWM value to 0 which is not
>> fall in the range, in VBIOS taken care this by limiting to 1.
>> This patch here will do the same. Either driver or VBIOS can not
>> pass 0 value as it is not a valid range for PWM and it will
>> give a high PWM pulse which is not the intented behaviour as
>> per HW constraints.
>>
>
>
> Comments from Windows dev, Anthony, CCed here as well:
>> In Windows, we typically never set backlight PWM value to 0.
>> Windows brightness slider is from 0 - 100% brightness.
>> 0% brightness maps to some non-zero value. So user never gets the chance to
>> set value to 0.
>>
>> I'm not 100% sure what kind of flashing they are seeing.
>> I thought 0 PWM works, and just means PWM signal always low, I think it
>> would equate to panel backlight completely being off.
>> I do know of a flashing issue caused by fractional PWM at low PWM values,
>> but this would not even be at 0. The flashing problem would be anywhere from
>> 1 - 300 for example. (out of 65535)
>
> Do you see flickering when value is at 0 or in the 1 > 0 transition? 1 would
> be 65535/256 = 256 and put us right in the 1-300 range that has a flashing
> problem.
>
> Harry
>
We can not make out if flicker is at transition from 1 to 0 or exactly
at 0, because the flickering is very instantaneous.But, I am not able to
see the issue @1 , only seen the issue after going down from 1.
Thanks,
Suresh G
>> Signed-off-by: suresh guttula
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
>> 1 file changed, 8 insertions(+)
>>
>> 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 492230c..38f84b2 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1518,6 +1518,14 @@ static int amdgpu_dm_backlight_update_status(struct
>> backlight_device *bd)
>> {
>> struct amdgpu_display_manager *dm = bl_get_data(bd);
>>
>> +/*
>> + * When we use brightness low key to reduce the brightness,
>> + * brightness level reaching to 0, with which we can see flash
>> + * screen on ui beacuse of HW limitation.To avoid that we are
>> + * limiting level to 1
>> + */
>> +if (bd->props.brightness < 1)
>> +return 1;
>> if (dc_link_set_backlight_level(dm->backlight_link,
>> bd->props.brightness, 0, 0))
>> return 0;
>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: set backlight level limit to 1
On 2018-10-25 2:58 a.m., Guttula, Suresh wrote:
> This patch will work as workaround for silicon limitation
> related to PWM dutycycle when the backlight level goes to 0.
>
> Actually PWM value is 16 bit value and valid range from 1-65535.
> when ever user requested to set this PWM value to 0 which is not
> fall in the range, in VBIOS taken care this by limiting to 1.
> This patch here will do the same. Either driver or VBIOS can not
> pass 0 value as it is not a valid range for PWM and it will
> give a high PWM pulse which is not the intented behaviour as
> per HW constraints.
>
Comments from Windows dev, Anthony, CCed here as well:
> In Windows, we typically never set backlight PWM value to 0.
> Windows brightness slider is from 0 - 100% brightness.
> 0% brightness maps to some non-zero value. So user never gets the chance to
> set value to 0.
>
> I'm not 100% sure what kind of flashing they are seeing.
> I thought 0 PWM works, and just means PWM signal always low, I think it would
> equate to panel backlight completely being off.
> I do know of a flashing issue caused by fractional PWM at low PWM values, but
> this would not even be at 0. The flashing problem would be anywhere from 1 -
> 300 for example. (out of 65535)
Do you see flickering when value is at 0 or in the 1 > 0 transition? 1 would be
65535/256 = 256 and put us right in the 1-300 range that has a flashing problem.
Harry
> Signed-off-by: suresh guttula
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8
> 1 file changed, 8 insertions(+)
>
> 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 492230c..38f84b2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1518,6 +1518,14 @@ static int amdgpu_dm_backlight_update_status(struct
> backlight_device *bd)
> {
> struct amdgpu_display_manager *dm = bl_get_data(bd);
>
> + /*
> + * When we use brightness low key to reduce the brightness,
> + * brightness level reaching to 0, with which we can see flash
> + * screen on ui beacuse of HW limitation.To avoid that we are
> + * limiting level to 1
> + */
> + if (bd->props.brightness < 1)
> + return 1;
> if (dc_link_set_backlight_level(dm->backlight_link,
> bd->props.brightness, 0, 0))
> return 0;
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
