Re: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

2019-10-22 Thread Kazlauskas, Nicholas
On 2019-10-22 9:33 a.m., Li, Roman wrote:
>> Any reason why we're skipping a flag here going from 0x2 to 0x8?
> 
> 0x4 is reserved for fractional pwm  mask:
> https://patchwork.freedesktop.org/patch/336923/

Ah, missed that patch. No problem then, feel free to go ahead with this.

Nicholas Kazlauskas

> 
> Thank you Nicholas for the review.
> 
> -Original Message-
> From: Kazlauskas, Nicholas 
> Sent: Tuesday, October 22, 2019 8:39 AM
> To: Li, Roman ; amd-gfx@lists.freedesktop.org; Deucher, 
> Alexander 
> Cc: Wentland, Harry ; Lakha, Bhawanpreet 
> ; Li, Sun peng (Leo) 
> Subject: Re: [PATCH] drm/amdgpu/display: add dc feature mask for psr 
> enablement
> 
> On 2019-10-21 5:45 p.m., roman...@amd.com wrote:
>> From: Roman Li 
>>
>> [Why]
>> Adding psr mask to dc features allows selectively disable/enable psr.
>> Current psr implementation may not work with non-pageflipping application.
>> Until resolved it should be disabled by default.
>>
>> [How]
>> Add dcfeaturemask for psr enablement. Disable by default.
>> To enable set amdgpu.dcfeaturemask=0x8 in grub kernel command line.
>>
>> Signed-off-by: Roman Li 
>> ---
>>drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>>drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>>2 files changed, 3 insertions(+), 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 1cf4beb..0f08879 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2424,7 +2424,8 @@ static int amdgpu_dm_initialize_drm_device(struct 
>> amdgpu_device *adev)
>>  } else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>>  amdgpu_dm_update_connector_after_detect(aconnector);
>>  register_backlight_device(dm, link);
>> -amdgpu_dm_set_psr_caps(link);
>> +if (amdgpu_dc_feature_mask & DC_PSR_MASK)
>> +amdgpu_dm_set_psr_caps(link);
>>  }
>>
>>
>> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
>> b/drivers/gpu/drm/amd/include/amd_shared.h
>> index 8889aac..1daa221 100644
>> --- a/drivers/gpu/drm/amd/include/amd_shared.h
>> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
>> @@ -143,6 +143,7 @@ enum PP_FEATURE_MASK {
>>enum DC_FEATURE_MASK {
>>  DC_FBC_MASK = 0x1,
>>  DC_MULTI_MON_PP_MCLK_SWITCH_MASK = 0x2,
>> +DC_PSR_MASK = 0x8,
> 
> Can this just be 0x4 instead? Any reason why we're skipping a flag here going 
> from 0x2 to 0x8?
> 
> You can still have my:
> 
> Reviewed-by: Nicholas Kazlauskas 
> 
> but my preference would be on fixing this up to a 0x4 first in the commit 
> message / DC_FEATURE_MASK.
> 
> Nicholas Kazlauskas
> 
>>};
>>
>>enum amd_dpm_forced_level;
>>
> 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

2019-10-22 Thread Li, Roman
> Any reason why we're skipping a flag here going from 0x2 to 0x8?

0x4 is reserved for fractional pwm  mask:
https://patchwork.freedesktop.org/patch/336923/

Thank you Nicholas for the review.

-Original Message-
From: Kazlauskas, Nicholas  
Sent: Tuesday, October 22, 2019 8:39 AM
To: Li, Roman ; amd-gfx@lists.freedesktop.org; Deucher, 
Alexander 
Cc: Wentland, Harry ; Lakha, Bhawanpreet 
; Li, Sun peng (Leo) 
Subject: Re: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

On 2019-10-21 5:45 p.m., roman...@amd.com wrote:
> From: Roman Li 
> 
> [Why]
> Adding psr mask to dc features allows selectively disable/enable psr.
> Current psr implementation may not work with non-pageflipping application.
> Until resolved it should be disabled by default.
> 
> [How]
> Add dcfeaturemask for psr enablement. Disable by default.
> To enable set amdgpu.dcfeaturemask=0x8 in grub kernel command line.
> 
> Signed-off-by: Roman Li 
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>   drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>   2 files changed, 3 insertions(+), 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 1cf4beb..0f08879 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2424,7 +2424,8 @@ static int amdgpu_dm_initialize_drm_device(struct 
> amdgpu_device *adev)
>   } else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>   amdgpu_dm_update_connector_after_detect(aconnector);
>   register_backlight_device(dm, link);
> - amdgpu_dm_set_psr_caps(link);
> + if (amdgpu_dc_feature_mask & DC_PSR_MASK)
> + amdgpu_dm_set_psr_caps(link);
>   }
>   
>   
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 8889aac..1daa221 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -143,6 +143,7 @@ enum PP_FEATURE_MASK {
>   enum DC_FEATURE_MASK {
>   DC_FBC_MASK = 0x1,
>   DC_MULTI_MON_PP_MCLK_SWITCH_MASK = 0x2,
> + DC_PSR_MASK = 0x8,

Can this just be 0x4 instead? Any reason why we're skipping a flag here going 
from 0x2 to 0x8?

You can still have my:

Reviewed-by: Nicholas Kazlauskas 

but my preference would be on fixing this up to a 0x4 first in the commit 
message / DC_FEATURE_MASK.

Nicholas Kazlauskas

>   };
>   
>   enum amd_dpm_forced_level;
> 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/display: add dc feature mask for psr enablement

2019-10-22 Thread Kazlauskas, Nicholas
On 2019-10-21 5:45 p.m., roman...@amd.com wrote:
> From: Roman Li 
> 
> [Why]
> Adding psr mask to dc features allows selectively disable/enable psr.
> Current psr implementation may not work with non-pageflipping application.
> Until resolved it should be disabled by default.
> 
> [How]
> Add dcfeaturemask for psr enablement. Disable by default.
> To enable set amdgpu.dcfeaturemask=0x8 in grub kernel command line.
> 
> Signed-off-by: Roman Li 
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>   drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>   2 files changed, 3 insertions(+), 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 1cf4beb..0f08879 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2424,7 +2424,8 @@ static int amdgpu_dm_initialize_drm_device(struct 
> amdgpu_device *adev)
>   } else if (dc_link_detect(link, DETECT_REASON_BOOT)) {
>   amdgpu_dm_update_connector_after_detect(aconnector);
>   register_backlight_device(dm, link);
> - amdgpu_dm_set_psr_caps(link);
> + if (amdgpu_dc_feature_mask & DC_PSR_MASK)
> + amdgpu_dm_set_psr_caps(link);
>   }
>   
>   
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 8889aac..1daa221 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -143,6 +143,7 @@ enum PP_FEATURE_MASK {
>   enum DC_FEATURE_MASK {
>   DC_FBC_MASK = 0x1,
>   DC_MULTI_MON_PP_MCLK_SWITCH_MASK = 0x2,
> + DC_PSR_MASK = 0x8,

Can this just be 0x4 instead? Any reason why we're skipping a flag here 
going from 0x2 to 0x8?

You can still have my:

Reviewed-by: Nicholas Kazlauskas 

but my preference would be on fixing this up to a 0x4 first in the 
commit message / DC_FEATURE_MASK.

Nicholas Kazlauskas

>   };
>   
>   enum amd_dpm_forced_level;
> 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx