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@xxxxxxxxx> > > Cc: Harry Wentland <harry.wentland@xxxxxxx> > Cc: Xaver Hugl <xaver.hugl@xxxxxxxxx> > Cc: Melissa Wen <mwen@xxxxxxxxxx> > --- > 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)