* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-01 18:28 [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR Joshua Ashton
@ 2024-01-01 21:36 ` Joshua Ashton
2024-01-02 16:53 ` Xaver Hugl
2024-01-03 14:35 ` Hamza Mahfooz
2024-03-12 15:44 ` Harry Wentland
2 siblings, 1 reply; 10+ messages in thread
From: Joshua Ashton @ 2024-01-01 21:36 UTC (permalink / raw)
To: amd-gfx; +Cc: Melissa Wen, Harry Wentland, Xaver Hugl
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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-01 21:36 ` Joshua Ashton
@ 2024-01-02 16:53 ` Xaver Hugl
0 siblings, 0 replies; 10+ messages in thread
From: Xaver Hugl @ 2024-01-02 16:53 UTC (permalink / raw)
To: Joshua Ashton; +Cc: Melissa Wen, Harry Wentland, amd-gfx
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-01 18:28 [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR Joshua Ashton
2024-01-01 21:36 ` Joshua Ashton
@ 2024-01-03 14:35 ` Hamza Mahfooz
2024-01-03 19:17 ` Joshua Ashton
2024-03-12 15:44 ` Harry Wentland
2 siblings, 1 reply; 10+ messages in thread
From: Hamza Mahfooz @ 2024-01-03 14:35 UTC (permalink / raw)
To: Joshua Ashton, amd-gfx; +Cc: Melissa Wen, Harry Wentland, Xaver Hugl
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-03 14:35 ` Hamza Mahfooz
@ 2024-01-03 19:17 ` Joshua Ashton
2024-01-04 13:54 ` Hamza Mahfooz
0 siblings, 1 reply; 10+ messages in thread
From: Joshua Ashton @ 2024-01-03 19:17 UTC (permalink / raw)
To: Hamza Mahfooz, amd-gfx
Cc: Melissa Wen, Deucher, Alexander, Harry Wentland, Xaver Hugl
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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-03 19:17 ` Joshua Ashton
@ 2024-01-04 13:54 ` Hamza Mahfooz
0 siblings, 0 replies; 10+ messages in thread
From: Hamza Mahfooz @ 2024-01-04 13:54 UTC (permalink / raw)
To: Joshua Ashton, amd-gfx
Cc: Melissa Wen, Deucher, Alexander, Harry Wentland, Xaver Hugl,
stable
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
@ 2024-01-04 13:54 ` Hamza Mahfooz
0 siblings, 0 replies; 10+ messages in thread
From: Hamza Mahfooz @ 2024-01-04 13:54 UTC (permalink / raw)
To: Joshua Ashton, amd-gfx
Cc: Melissa Wen, Harry Wentland, Xaver Hugl, Deucher, Alexander,
stable
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-04 13:54 ` Hamza Mahfooz
@ 2024-01-04 13:58 ` Greg KH
-1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2024-01-04 13:58 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: Xaver Hugl, amd-gfx, Melissa Wen, stable, Deucher, Alexander,
Harry Wentland, Joshua Ashton
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>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
@ 2024-01-04 13:58 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2024-01-04 13:58 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: Joshua Ashton, amd-gfx, Melissa Wen, Harry Wentland, Xaver Hugl,
Deucher, Alexander, stable
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR
2024-01-01 18:28 [PATCH] drm/amd/display: Fix sending VSC (+ colorimetry) packets for DP/eDP displays without PSR Joshua Ashton
2024-01-01 21:36 ` Joshua Ashton
2024-01-03 14:35 ` Hamza Mahfooz
@ 2024-03-12 15:44 ` Harry Wentland
2 siblings, 0 replies; 10+ messages in thread
From: Harry Wentland @ 2024-03-12 15:44 UTC (permalink / raw)
To: Joshua Ashton, amd-gfx; +Cc: Xaver Hugl, Melissa Wen
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)
^ permalink raw reply [flat|nested] 10+ messages in thread