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 <jos...@froggi.es>
>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Cc: Xaver Hugl <xaver.h...@gmail.com>
> Cc: Melissa Wen <m...@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)
