| Message ID | 20231025121318.2732051-5-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DP DSC min/max src bpc fixes | expand |
On Wed, Oct 25, 2023 at 05:43:18PM +0530, Ankit Nautiyal wrote: > At the moment the max requested bpc is limited to 6 to 10/12. > For platforms that support DSC, min and max src bpc with DSC are > different. > > Account for DSC bpc limitations, when setting min and max value for > max_requested_bpc property. NAK. DSC capabiliies change dynamically, the property does not. > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index abc718f1a878..1935b9014b12 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5907,8 +5907,17 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > intel_attach_broadcast_rgb_property(connector); > if (HAS_GMCH(dev_priv)) > drm_connector_attach_max_bpc_property(connector, 6, 10); > - else if (DISPLAY_VER(dev_priv) >= 5) > - drm_connector_attach_max_bpc_property(connector, 6, 12); > + else if (DISPLAY_VER(dev_priv) >= 5) { > + int min_bpc = 6, max_bpc = 12; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + > + if (HAS_DSC(dev_priv) && drm_dp_sink_supports_dsc(intel_connector->dp.dsc_dpcd)) { > + min_bpc = intel_dp_dsc_min_src_input_bpc(dev_priv); > + max_bpc = intel_dp_dsc_max_src_input_bpc(dev_priv); > + } > + > + drm_connector_attach_max_bpc_property(connector, min_bpc, max_bpc); > + } > > /* Register HDMI colorspace for case of lspcon */ > if (intel_bios_encoder_is_lspcon(dp_to_dig_port(intel_dp)->base.devdata)) { > -- > 2.40.1
On 10/25/2023 6:20 PM, Ville Syrjälä wrote: > On Wed, Oct 25, 2023 at 05:43:18PM +0530, Ankit Nautiyal wrote: >> At the moment the max requested bpc is limited to 6 to 10/12. >> For platforms that support DSC, min and max src bpc with DSC are >> different. >> >> Account for DSC bpc limitations, when setting min and max value for >> max_requested_bpc property. > NAK. DSC capabiliies change dynamically, the property does not. Hmm, perhaps I should remove the check for sink DSC support and have only Platform check HAS_DSC. The problem I am trying to fix is that our HW does not support DSC with 6bpc, but we are allowing the max_requested_bpc to be 6 bpc. This can be a problem with some eDP panels that support modes like 4k@120 which will always need DSC and when max requested bpc property is set to 6. I am wondering how to avoid this. Does it make sense to have the min value for the max_requested_bpc to be 8, for platforms that support DSC? Regards, Ankit > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c >> index abc718f1a878..1935b9014b12 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> @@ -5907,8 +5907,17 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect >> intel_attach_broadcast_rgb_property(connector); >> if (HAS_GMCH(dev_priv)) >> drm_connector_attach_max_bpc_property(connector, 6, 10); >> - else if (DISPLAY_VER(dev_priv) >= 5) >> - drm_connector_attach_max_bpc_property(connector, 6, 12); >> + else if (DISPLAY_VER(dev_priv) >= 5) { >> + int min_bpc = 6, max_bpc = 12; >> + struct intel_connector *intel_connector = to_intel_connector(connector); >> + >> + if (HAS_DSC(dev_priv) && drm_dp_sink_supports_dsc(intel_connector->dp.dsc_dpcd)) { >> + min_bpc = intel_dp_dsc_min_src_input_bpc(dev_priv); >> + max_bpc = intel_dp_dsc_max_src_input_bpc(dev_priv); >> + } >> + >> + drm_connector_attach_max_bpc_property(connector, min_bpc, max_bpc); >> + } >> >> /* Register HDMI colorspace for case of lspcon */ >> if (intel_bios_encoder_is_lspcon(dp_to_dig_port(intel_dp)->base.devdata)) { >> -- >> 2.40.1
On Thu, Oct 26, 2023 at 11:24:38AM +0530, Nautiyal, Ankit K wrote: > > On 10/25/2023 6:20 PM, Ville Syrjälä wrote: > > On Wed, Oct 25, 2023 at 05:43:18PM +0530, Ankit Nautiyal wrote: > >> At the moment the max requested bpc is limited to 6 to 10/12. > >> For platforms that support DSC, min and max src bpc with DSC are > >> different. > >> > >> Account for DSC bpc limitations, when setting min and max value for > >> max_requested_bpc property. > > NAK. DSC capabiliies change dynamically, the property does not. > > Hmm, perhaps I should remove the check for sink DSC support and have > only Platform check HAS_DSC. > > The problem I am trying to fix is that our HW does not support DSC with > 6bpc, but we are allowing the max_requested_bpc to be 6 bpc. > > This can be a problem with some eDP panels that support modes like > 4k@120 which will always need DSC and when max requested bpc property is > set to 6. > > I am wondering how to avoid this. Does it make sense to have the min > value for the max_requested_bpc to be 8, for platforms that support DSC? I think the easy fix is to just use 'min_bpc = max(dsc_min_bpc, max_bpc*3)' with DSC. And likely we should do something like that for YCbCr output as well.
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index abc718f1a878..1935b9014b12 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5907,8 +5907,17 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect intel_attach_broadcast_rgb_property(connector); if (HAS_GMCH(dev_priv)) drm_connector_attach_max_bpc_property(connector, 6, 10); - else if (DISPLAY_VER(dev_priv) >= 5) - drm_connector_attach_max_bpc_property(connector, 6, 12); + else if (DISPLAY_VER(dev_priv) >= 5) { + int min_bpc = 6, max_bpc = 12; + struct intel_connector *intel_connector = to_intel_connector(connector); + + if (HAS_DSC(dev_priv) && drm_dp_sink_supports_dsc(intel_connector->dp.dsc_dpcd)) { + min_bpc = intel_dp_dsc_min_src_input_bpc(dev_priv); + max_bpc = intel_dp_dsc_max_src_input_bpc(dev_priv); + } + + drm_connector_attach_max_bpc_property(connector, min_bpc, max_bpc); + } /* Register HDMI colorspace for case of lspcon */ if (intel_bios_encoder_is_lspcon(dp_to_dig_port(intel_dp)->base.devdata)) {
At the moment the max requested bpc is limited to 6 to 10/12. For platforms that support DSC, min and max src bpc with DSC are different. Account for DSC bpc limitations, when setting min and max value for max_requested_bpc property. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)