[PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Joshua Ashton]

The check for sending the vsc infopacket to the display was gated behind
PSR (Panel Self Refresh) being enabled.

The vsc infopacket also contains the colorimetry (specifically the
container color gamut) information for the stream on modern DP.

PSR is typically only supported on mobile phone eDP displays, thus this
was not getting sent for typical desktop monitors or TV screens.

This functionality is needed for proper HDR10 functionality on DP as it
wants BT2020 RGB/YCbCr for the container color space.

Signed-off-by: Joshua Ashton <joshua@froggi.es>

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Xaver Hugl <xaver.hugl@gmail.com>
Cc: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
 .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

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 2845c884398e..6dff56408bf4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector *connector,
 
 	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
 		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
-
-	if (stream->link->psr_settings.psr_feature_enabled || stream->link->replay_settings.replay_feature_enabled) {
+	else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
+			 stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
+			 stream->signal == SIGNAL_TYPE_EDP) {
 		//
 		// should decide stream support vsc sdp colorimetry capability
 		// before building vsc info packet
@@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector *connector,
 		if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
 			tf = TRANSFER_FUNC_GAMMA_22;
 		mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, stream->output_color_space, tf);
-		aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
 
+		if (stream->link->psr_settings.psr_feature_enabled)
+			aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
 	}
 finish:
 	dc_sink_release(sink);
diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
index 84f9b412a4f1..738ee763f24a 100644
--- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
+++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
@@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
 	}
 
 	/* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
-	if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
-		vsc_packet_revision = vsc_packet_rev4;
-	else if (stream->link->replay_settings.config.replay_supported)
+	if (stream->link->psr_settings.psr_feature_enabled) {
+		if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
+			vsc_packet_revision = vsc_packet_rev4;
+		else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
+			vsc_packet_revision = vsc_packet_rev2;
+	}
+
+	if (stream->link->replay_settings.config.replay_supported)
 		vsc_packet_revision = vsc_packet_rev4;
-	else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
-		vsc_packet_revision = vsc_packet_rev2;
 
 	/* Update to revision 5 for extended colorimetry support */
 	if (stream->use_vsc_sdp_for_colorimetry)
-- 
2.43.0

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Joshua Ashton]

 From the issue:

```
Thank you for for fixing this!
I built a custom kernel with this patch on the fedora rawhide kernel 
(6.7.0-0.rc8.61.fc40.x86_64) and now the colors look correct. SDR 
content is now displayed as sRGB and HDR/WCG content can use the full 
capabilities of the display.
I currently don't have a desktop mail client installed to comment on the 
mailing list directly, so I'll post it here (not sure if it counts or 
matters 😀 )

Tested-By: Simon Berz <simon@berz.me>
```

- Joshie 🐸✨

On 1/1/24 18:28, Joshua Ashton wrote:
> The check for sending the vsc infopacket to the display was gated behind
> PSR (Panel Self Refresh) being enabled.
> 
> The vsc infopacket also contains the colorimetry (specifically the
> container color gamut) information for the stream on modern DP.
> 
> PSR is typically only supported on mobile phone eDP displays, thus this
> was not getting sent for typical desktop monitors or TV screens.
> 
> This functionality is needed for proper HDR10 functionality on DP as it
> wants BT2020 RGB/YCbCr for the container color space.
> 
> Signed-off-by: Joshua Ashton <joshua@froggi.es>
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> Cc: Melissa Wen <mwen@igalia.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>   2 files changed, 13 insertions(+), 8 deletions(-)
> 
> 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 2845c884398e..6dff56408bf4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector *connector,
>   
>   	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>   		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
> -
> -	if (stream->link->psr_settings.psr_feature_enabled || stream->link->replay_settings.replay_feature_enabled) {
> +	else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
> +			 stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
> +			 stream->signal == SIGNAL_TYPE_EDP) {
>   		//
>   		// should decide stream support vsc sdp colorimetry capability
>   		// before building vsc info packet
> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector *connector,
>   		if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
>   			tf = TRANSFER_FUNC_GAMMA_22;
>   		mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, stream->output_color_space, tf);
> -		aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>   
> +		if (stream->link->psr_settings.psr_feature_enabled)
> +			aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>   	}
>   finish:
>   	dc_sink_release(sink);
> diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> index 84f9b412a4f1..738ee763f24a 100644
> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
>   	}
>   
>   	/* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
> -	if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> -		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->replay_settings.config.replay_supported)
> +	if (stream->link->psr_settings.psr_feature_enabled) {
> +		if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> +			vsc_packet_revision = vsc_packet_rev4;
> +		else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> +			vsc_packet_revision = vsc_packet_rev2;
> +	}
> +
> +	if (stream->link->replay_settings.config.replay_supported)
>   		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> -		vsc_packet_revision = vsc_packet_rev2;
>   
>   	/* Update to revision 5 for extended colorimetry support */
>   	if (stream->use_vsc_sdp_for_colorimetry)

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Xaver Hugl]

[-- Attachment #1: Type: text/plain, Size: 4978 bytes --]

Hi,

I tested the patch and it fixes the issue for me too. Consider it
Tested-By Xaver Hugl <xaver.hugl@kde.org>

- Xaver

Am Mo., 1. Jan. 2024 um 22:37 Uhr schrieb Joshua Ashton <joshua@froggi.es>:

>  From the issue:
>
> ```
> Thank you for for fixing this!
> I built a custom kernel with this patch on the fedora rawhide kernel
> (6.7.0-0.rc8.61.fc40.x86_64) and now the colors look correct. SDR
> content is now displayed as sRGB and HDR/WCG content can use the full
> capabilities of the display.
> I currently don't have a desktop mail client installed to comment on the
> mailing list directly, so I'll post it here (not sure if it counts or
> matters 😀 )
>
> Tested-By: Simon Berz <simon@berz.me>
> ```
>
> - Joshie 🐸✨
>
> On 1/1/24 18:28, Joshua Ashton wrote:
> > The check for sending the vsc infopacket to the display was gated behind
> > PSR (Panel Self Refresh) being enabled.
> >
> > The vsc infopacket also contains the colorimetry (specifically the
> > container color gamut) information for the stream on modern DP.
> >
> > PSR is typically only supported on mobile phone eDP displays, thus this
> > was not getting sent for typical desktop monitors or TV screens.
> >
> > This functionality is needed for proper HDR10 functionality on DP as it
> > wants BT2020 RGB/YCbCr for the container color space.
> >
> > Signed-off-by: Joshua Ashton <joshua@froggi.es>
> >
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Xaver Hugl <xaver.hugl@gmail.com>
> > Cc: Melissa Wen <mwen@igalia.com>
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
> >   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
> >   2 files changed, 13 insertions(+), 8 deletions(-)
> >
> > 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 2845c884398e..6dff56408bf4 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector
> *connector,
> >
> >       if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
> >               mod_build_hf_vsif_infopacket(stream,
> &stream->vsp_infopacket);
> > -
> > -     if (stream->link->psr_settings.psr_feature_enabled ||
> stream->link->replay_settings.replay_feature_enabled) {
> > +     else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
> > +                      stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
> > +                      stream->signal == SIGNAL_TYPE_EDP) {
> >               //
> >               // should decide stream support vsc sdp colorimetry
> capability
> >               // before building vsc info packet
> > @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector
> *connector,
> >               if (stream->out_transfer_func->tf ==
> TRANSFER_FUNCTION_GAMMA22)
> >                       tf = TRANSFER_FUNC_GAMMA_22;
> >               mod_build_vsc_infopacket(stream, &stream->vsc_infopacket,
> stream->output_color_space, tf);
> > -             aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
> >
> > +             if (stream->link->psr_settings.psr_feature_enabled)
> > +                     aconnector->psr_skip_count =
> AMDGPU_DM_PSR_ENTRY_DELAY;
> >       }
> >   finish:
> >       dc_sink_release(sink);
> > diff --git
> a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> > index 84f9b412a4f1..738ee763f24a 100644
> > --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> > +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> > @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct
> dc_stream_state *stream,
> >       }
> >
> >       /* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
> > -     if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> > -             vsc_packet_revision = vsc_packet_rev4;
> > -     else if (stream->link->replay_settings.config.replay_supported)
> > +     if (stream->link->psr_settings.psr_feature_enabled) {
> > +             if (stream->link->psr_settings.psr_version ==
> DC_PSR_VERSION_SU_1)
> > +                     vsc_packet_revision = vsc_packet_rev4;
> > +             else if (stream->link->psr_settings.psr_version ==
> DC_PSR_VERSION_1)
> > +                     vsc_packet_revision = vsc_packet_rev2;
> > +     }
> > +
> > +     if (stream->link->replay_settings.config.replay_supported)
> >               vsc_packet_revision = vsc_packet_rev4;
> > -     else if (stream->link->psr_settings.psr_version ==
> DC_PSR_VERSION_1)
> > -             vsc_packet_revision = vsc_packet_rev2;
> >
> >       /* Update to revision 5 for extended colorimetry support */
> >       if (stream->use_vsc_sdp_for_colorimetry)
>
>

[-- Attachment #2: Type: text/html, Size: 6410 bytes --]

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Hamza Mahfooz]

On 1/1/24 13:28, Joshua Ashton wrote:
> The check for sending the vsc infopacket to the display was gated behind
> PSR (Panel Self Refresh) being enabled.
> 
> The vsc infopacket also contains the colorimetry (specifically the
> container color gamut) information for the stream on modern DP.
> 
> PSR is typically only supported on mobile phone eDP displays, thus this
> was not getting sent for typical desktop monitors or TV screens.
> 
> This functionality is needed for proper HDR10 functionality on DP as it
> wants BT2020 RGB/YCbCr for the container color space.
> 
> Signed-off-by: Joshua Ashton <joshua@froggi.es>
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> Cc: Melissa Wen <mwen@igalia.com>

Applied, thanks!

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>   2 files changed, 13 insertions(+), 8 deletions(-)
> 
> 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 2845c884398e..6dff56408bf4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector *connector,
>   
>   	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>   		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
> -
> -	if (stream->link->psr_settings.psr_feature_enabled || stream->link->replay_settings.replay_feature_enabled) {
> +	else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
> +			 stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
> +			 stream->signal == SIGNAL_TYPE_EDP) {
>   		//
>   		// should decide stream support vsc sdp colorimetry capability
>   		// before building vsc info packet
> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector *connector,
>   		if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
>   			tf = TRANSFER_FUNC_GAMMA_22;
>   		mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, stream->output_color_space, tf);
> -		aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>   
> +		if (stream->link->psr_settings.psr_feature_enabled)
> +			aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>   	}
>   finish:
>   	dc_sink_release(sink);
> diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> index 84f9b412a4f1..738ee763f24a 100644
> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
>   	}
>   
>   	/* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
> -	if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> -		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->replay_settings.config.replay_supported)
> +	if (stream->link->psr_settings.psr_feature_enabled) {
> +		if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> +			vsc_packet_revision = vsc_packet_rev4;
> +		else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> +			vsc_packet_revision = vsc_packet_rev2;
> +	}
> +
> +	if (stream->link->replay_settings.config.replay_supported)
>   		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> -		vsc_packet_revision = vsc_packet_rev2;
>   
>   	/* Update to revision 5 for extended colorimetry support */
>   	if (stream->use_vsc_sdp_for_colorimetry)
-- 
Hamza

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Joshua Ashton]

Thanks! Is it possible for us to get this backported too?

I forgot to add a Fixes: tag to this commit. It should be

Fixes: 15f9dfd545a1 ("drm/amd/display: Register Colorspace property for 
DP and HDMI")

- Joshie 🐸✨

On 1/3/24 14:35, Hamza Mahfooz wrote:
> On 1/1/24 13:28, Joshua Ashton wrote:
>> The check for sending the vsc infopacket to the display was gated behind
>> PSR (Panel Self Refresh) being enabled.
>>
>> The vsc infopacket also contains the colorimetry (specifically the
>> container color gamut) information for the stream on modern DP.
>>
>> PSR is typically only supported on mobile phone eDP displays, thus this
>> was not getting sent for typical desktop monitors or TV screens.
>>
>> This functionality is needed for proper HDR10 functionality on DP as it
>> wants BT2020 RGB/YCbCr for the container color space.
>>
>> Signed-off-by: Joshua Ashton <joshua@froggi.es>
>>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Xaver Hugl <xaver.hugl@gmail.com>
>> Cc: Melissa Wen <mwen@igalia.com>
> 
> Applied, thanks!
> 
>> ---
>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>>   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>>   2 files changed, 13 insertions(+), 8 deletions(-)
>>
>> 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 2845c884398e..6dff56408bf4 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector 
>> *connector,
>>       if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>>           mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
>> -
>> -    if (stream->link->psr_settings.psr_feature_enabled || 
>> stream->link->replay_settings.replay_feature_enabled) {
>> +    else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
>> +             stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
>> +             stream->signal == SIGNAL_TYPE_EDP) {
>>           //
>>           // should decide stream support vsc sdp colorimetry capability
>>           // before building vsc info packet
>> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector 
>> *connector,
>>           if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
>>               tf = TRANSFER_FUNC_GAMMA_22;
>>           mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, 
>> stream->output_color_space, tf);
>> -        aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>> +        if (stream->link->psr_settings.psr_feature_enabled)
>> +            aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>>       }
>>   finish:
>>       dc_sink_release(sink);
>> diff --git 
>> a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c 
>> b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>> index 84f9b412a4f1..738ee763f24a 100644
>> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct 
>> dc_stream_state *stream,
>>       }
>>       /* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
>> -    if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
>> -        vsc_packet_revision = vsc_packet_rev4;
>> -    else if (stream->link->replay_settings.config.replay_supported)
>> +    if (stream->link->psr_settings.psr_feature_enabled) {
>> +        if (stream->link->psr_settings.psr_version == 
>> DC_PSR_VERSION_SU_1)
>> +            vsc_packet_revision = vsc_packet_rev4;
>> +        else if (stream->link->psr_settings.psr_version == 
>> DC_PSR_VERSION_1)
>> +            vsc_packet_revision = vsc_packet_rev2;
>> +    }
>> +
>> +    if (stream->link->replay_settings.config.replay_supported)
>>           vsc_packet_revision = vsc_packet_rev4;
>> -    else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
>> -        vsc_packet_revision = vsc_packet_rev2;
>>       /* Update to revision 5 for extended colorimetry support */
>>       if (stream->use_vsc_sdp_for_colorimetry)

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Hamza Mahfooz]

On 1/3/24 14:17, Joshua Ashton wrote:
> Thanks! Is it possible for us to get this backported too?

Sure thing.

Cc: stable@vger.kernel.org

> 
> I forgot to add a Fixes: tag to this commit. It should be
> 
> Fixes: 15f9dfd545a1 ("drm/amd/display: Register Colorspace property for 
> DP and HDMI")
> 
> - Joshie 🐸✨
> 
> On 1/3/24 14:35, Hamza Mahfooz wrote:
>> On 1/1/24 13:28, Joshua Ashton wrote:
>>> The check for sending the vsc infopacket to the display was gated behind
>>> PSR (Panel Self Refresh) being enabled.
>>>
>>> The vsc infopacket also contains the colorimetry (specifically the
>>> container color gamut) information for the stream on modern DP.
>>>
>>> PSR is typically only supported on mobile phone eDP displays, thus this
>>> was not getting sent for typical desktop monitors or TV screens.
>>>
>>> This functionality is needed for proper HDR10 functionality on DP as it
>>> wants BT2020 RGB/YCbCr for the container color space.
>>>
>>> Signed-off-by: Joshua Ashton <joshua@froggi.es>
>>>
>>> Cc: Harry Wentland <harry.wentland@amd.com>
>>> Cc: Xaver Hugl <xaver.hugl@gmail.com>
>>> Cc: Melissa Wen <mwen@igalia.com>
>>
>> Applied, thanks!
>>
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>>>   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>>>   2 files changed, 13 insertions(+), 8 deletions(-)
>>>
>>> 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 2845c884398e..6dff56408bf4 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector 
>>> *connector,
>>>       if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>>>           mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
>>> -
>>> -    if (stream->link->psr_settings.psr_feature_enabled || 
>>> stream->link->replay_settings.replay_feature_enabled) {
>>> +    else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
>>> +             stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
>>> +             stream->signal == SIGNAL_TYPE_EDP) {
>>>           //
>>>           // should decide stream support vsc sdp colorimetry capability
>>>           // before building vsc info packet
>>> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector 
>>> *connector,
>>>           if (stream->out_transfer_func->tf == 
>>> TRANSFER_FUNCTION_GAMMA22)
>>>               tf = TRANSFER_FUNC_GAMMA_22;
>>>           mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, 
>>> stream->output_color_space, tf);
>>> -        aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>>> +        if (stream->link->psr_settings.psr_feature_enabled)
>>> +            aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>>>       }
>>>   finish:
>>>       dc_sink_release(sink);
>>> diff --git 
>>> a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c 
>>> b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> index 84f9b412a4f1..738ee763f24a 100644
>>> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct 
>>> dc_stream_state *stream,
>>>       }
>>>       /* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
>>> -    if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
>>> -        vsc_packet_revision = vsc_packet_rev4;
>>> -    else if (stream->link->replay_settings.config.replay_supported)
>>> +    if (stream->link->psr_settings.psr_feature_enabled) {
>>> +        if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_SU_1)
>>> +            vsc_packet_revision = vsc_packet_rev4;
>>> +        else if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_1)
>>> +            vsc_packet_revision = vsc_packet_rev2;
>>> +    }
>>> +
>>> +    if (stream->link->replay_settings.config.replay_supported)
>>>           vsc_packet_revision = vsc_packet_rev4;
>>> -    else if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_1)
>>> -        vsc_packet_revision = vsc_packet_rev2;
>>>       /* Update to revision 5 for extended colorimetry support */
>>>       if (stream->use_vsc_sdp_for_colorimetry)
> 
-- 
Hamza

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Hamza Mahfooz]

On 1/3/24 14:17, Joshua Ashton wrote:
> Thanks! Is it possible for us to get this backported too?

Sure thing.

Cc: stable@vger.kernel.org

> 
> I forgot to add a Fixes: tag to this commit. It should be
> 
> Fixes: 15f9dfd545a1 ("drm/amd/display: Register Colorspace property for 
> DP and HDMI")
> 
> - Joshie 🐸✨
> 
> On 1/3/24 14:35, Hamza Mahfooz wrote:
>> On 1/1/24 13:28, Joshua Ashton wrote:
>>> The check for sending the vsc infopacket to the display was gated behind
>>> PSR (Panel Self Refresh) being enabled.
>>>
>>> The vsc infopacket also contains the colorimetry (specifically the
>>> container color gamut) information for the stream on modern DP.
>>>
>>> PSR is typically only supported on mobile phone eDP displays, thus this
>>> was not getting sent for typical desktop monitors or TV screens.
>>>
>>> This functionality is needed for proper HDR10 functionality on DP as it
>>> wants BT2020 RGB/YCbCr for the container color space.
>>>
>>> Signed-off-by: Joshua Ashton <joshua@froggi.es>
>>>
>>> Cc: Harry Wentland <harry.wentland@amd.com>
>>> Cc: Xaver Hugl <xaver.hugl@gmail.com>
>>> Cc: Melissa Wen <mwen@igalia.com>
>>
>> Applied, thanks!
>>
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>>>   .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>>>   2 files changed, 13 insertions(+), 8 deletions(-)
>>>
>>> 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 2845c884398e..6dff56408bf4 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector 
>>> *connector,
>>>       if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>>>           mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
>>> -
>>> -    if (stream->link->psr_settings.psr_feature_enabled || 
>>> stream->link->replay_settings.replay_feature_enabled) {
>>> +    else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
>>> +             stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
>>> +             stream->signal == SIGNAL_TYPE_EDP) {
>>>           //
>>>           // should decide stream support vsc sdp colorimetry capability
>>>           // before building vsc info packet
>>> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector 
>>> *connector,
>>>           if (stream->out_transfer_func->tf == 
>>> TRANSFER_FUNCTION_GAMMA22)
>>>               tf = TRANSFER_FUNC_GAMMA_22;
>>>           mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, 
>>> stream->output_color_space, tf);
>>> -        aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>>> +        if (stream->link->psr_settings.psr_feature_enabled)
>>> +            aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>>>       }
>>>   finish:
>>>       dc_sink_release(sink);
>>> diff --git 
>>> a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c 
>>> b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> index 84f9b412a4f1..738ee763f24a 100644
>>> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
>>> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct 
>>> dc_stream_state *stream,
>>>       }
>>>       /* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
>>> -    if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
>>> -        vsc_packet_revision = vsc_packet_rev4;
>>> -    else if (stream->link->replay_settings.config.replay_supported)
>>> +    if (stream->link->psr_settings.psr_feature_enabled) {
>>> +        if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_SU_1)
>>> +            vsc_packet_revision = vsc_packet_rev4;
>>> +        else if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_1)
>>> +            vsc_packet_revision = vsc_packet_rev2;
>>> +    }
>>> +
>>> +    if (stream->link->replay_settings.config.replay_supported)
>>>           vsc_packet_revision = vsc_packet_rev4;
>>> -    else if (stream->link->psr_settings.psr_version == 
>>> DC_PSR_VERSION_1)
>>> -        vsc_packet_revision = vsc_packet_rev2;
>>>       /* Update to revision 5 for extended colorimetry support */
>>>       if (stream->use_vsc_sdp_for_colorimetry)
> 
-- 
Hamza

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Greg KH]

On Thu, Jan 04, 2024 at 08:54:19AM -0500, Hamza Mahfooz wrote:
> On 1/3/24 14:17, Joshua Ashton wrote:
> > Thanks! Is it possible for us to get this backported too?
> 
> Sure thing.
> 
> Cc: stable@vger.kernel.org

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Greg KH]

On Thu, Jan 04, 2024 at 08:54:19AM -0500, Hamza Mahfooz wrote:
> On 1/3/24 14:17, Joshua Ashton wrote:
> > Thanks! Is it possible for us to get this backported too?
> 
> Sure thing.
> 
> Cc: stable@vger.kernel.org

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR

[Harry Wentland]

On 2024-01-01 13:28, Joshua Ashton wrote:
> The check for sending the vsc infopacket to the display was gated behind
> PSR (Panel Self Refresh) being enabled.
> 
> The vsc infopacket also contains the colorimetry (specifically the
> container color gamut) information for the stream on modern DP.
> 
> PSR is typically only supported on mobile phone eDP displays, thus this
> was not getting sent for typical desktop monitors or TV screens.
> 
> This functionality is needed for proper HDR10 functionality on DP as it
> wants BT2020 RGB/YCbCr for the container color space.
> 

So apparently this caused regressions on some panels. I sent a revert
and we'll need to revisit this.

https://gitlab.freedesktop.org/drm/amd/-/issues/3207
https://gitlab.freedesktop.org/drm/amd/-/issues/3151

> Signed-off-by: Joshua Ashton <joshua@froggi.es>
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Xaver Hugl <xaver.hugl@gmail.com>
> Cc: Melissa Wen <mwen@igalia.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  8 +++++---
>  .../amd/display/modules/info_packet/info_packet.c   | 13 ++++++++-----
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> 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 2845c884398e..6dff56408bf4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6233,8 +6233,9 @@ create_stream_for_sink(struct drm_connector *connector,
>  
>  	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>  		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket);
> -
> -	if (stream->link->psr_settings.psr_feature_enabled || stream->link->replay_settings.replay_feature_enabled) {
> +	else if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT ||
> +			 stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
> +			 stream->signal == SIGNAL_TYPE_EDP) {
>  		//
>  		// should decide stream support vsc sdp colorimetry capability
>  		// before building vsc info packet

The use_vsc_sdp_for_colorimetry is being cut off in this patch
since it's not changed. We should add a DPCD revision check to
ensure the panel's revision is >= 1.4. This is what we do on other
OSes and this is likely why we're seeing the regressions in the
freedesktop issues.

> 			if (stream->linkstream->link->dpcd_caps.dprx_feature.bits.VSC_SDP_COLORIMETRY_SUPPORTED)
> 				stream->use_vsc_sdp_for_colorimetry = true;



> @@ -6250,8 +6251,9 @@ create_stream_for_sink(struct drm_connector *connector,
>  		if (stream->out_transfer_func->tf == TRANSFER_FUNCTION_GAMMA22)
>  			tf = TRANSFER_FUNC_GAMMA_22;
>  		mod_build_vsc_infopacket(stream, &stream->vsc_infopacket, stream->output_color_space, tf);
> -		aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>  
> +		if (stream->link->psr_settings.psr_feature_enabled)
> +			aconnector->psr_skip_count = AMDGPU_DM_PSR_ENTRY_DELAY;
>  	}
>  finish:
>  	dc_sink_release(sink);
> diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> index 84f9b412a4f1..738ee763f24a 100644
> --- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> +++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
> @@ -147,12 +147,15 @@ void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
>  	}
>  
>  	/* VSC packet set to 4 for PSR-SU, or 2 for PSR1 */
> -	if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> -		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->replay_settings.config.replay_supported)
> +	if (stream->link->psr_settings.psr_feature_enabled) {
> +		if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_SU_1)
> +			vsc_packet_revision = vsc_packet_rev4;
> +		else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> +			vsc_packet_revision = vsc_packet_rev2;
> +	}
> +
> +	if (stream->link->replay_settings.config.replay_supported)
>  		vsc_packet_revision = vsc_packet_rev4;
> -	else if (stream->link->psr_settings.psr_version == DC_PSR_VERSION_1)
> -		vsc_packet_revision = vsc_packet_rev2;
>  

I'm curious whether this is really needed? The original code
should already do the same, even without the additional
.psr_feature_enabled check.

I'll send a patch that is intended to fix the colorimetry while
trying to avoid the regressions.

Harry

>  	/* Update to revision 5 for extended colorimetry support */
>  	if (stream->use_vsc_sdp_for_colorimetry)