[PATCH] drm/amd/display: set backlight level limit to 1

[PATCH] drm/amd/display: set backlight level limit to 1

[Guttula, Suresh]

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.

Signed-off-by: suresh guttula <suresh.guttula@amd.com>
---
 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;
-- 
2.7.4

_______________________________________________
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

[Wentland, Harry]

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 <suresh.guttula@amd.com>
> ---
>  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

[Guttula-CC+yJ3UmIYqDUpFQwHEjaQ]

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 <suresh.guttula@amd.com>
>> ---
>>   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

[Wentland, Harry]

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.wentland@amd.com>

Harry

> Thanks,
> Suresh G
> 
>>> Signed-off-by: suresh guttula <suresh.guttula@amd.com>
>>> ---
>>>   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