| Message ID | 20220324110959.1857869-1-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/display: Extend DP HDR support to hsw+ | expand |
On Thu, Mar 24, 2022 at 04:39:59PM +0530, Uma Shankar wrote: > HSW+ platforms are able to send out HDR Metadata SDP DIP > packet as GMP. Hence, extending the support for HDR on DP > encoders for the same. > > v2: Limited to non eDP ports on hsw/bdw and removed it for > lspcon as it is done separately (suggested by Ville) > > v3: Added helper and limited eDP restriction to port A (Ville) > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5389 > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 9e19165fd175..c993d82c7ec9 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4913,6 +4913,26 @@ bool intel_dp_is_port_edp(struct drm_i915_private *dev_priv, enum port port) > return intel_bios_is_port_edp(dev_priv, port); > } > > +static bool > +has_gamut_metadata_dip(struct drm_i915_private *dev_priv, s/dev_priv/i915/ for modern style > + struct intel_dp *intel_dp, enum port port) > +{ > + if (intel_bios_is_lspcon_present(dev_priv, port)) > + return false; > + > + if (DISPLAY_VER(dev_priv) >= 10 && !IS_GEMINILAKE(dev_priv)) > + return true; DISPLAY_VER >= 11 > + > + if (port == PORT_A && intel_dp_is_edp(intel_dp)) The is_edp check is still wrong. Should be just port==A. Also allows you to drop the intel_dp argument to the function. > + return false; > + > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || > + IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 9) > + return true; The IS_GLK check is redundant. > + > + return false; > +} > + > static void > intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector) > { > @@ -4939,7 +4959,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > intel_attach_dp_colorspace_property(connector); > } > > - if (IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 11) > + if (has_gamut_metadata_dip(dev_priv, intel_dp, port)) > drm_object_attach_property(&connector->base, > connector->dev->mode_config.hdr_output_metadata_property, > 0); > -- > 2.25.1
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, March 24, 2022 5:12 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [v3] drm/i915/display: Extend DP HDR support to hsw+ > > On Thu, Mar 24, 2022 at 04:39:59PM +0530, Uma Shankar wrote: > > HSW+ platforms are able to send out HDR Metadata SDP DIP > > packet as GMP. Hence, extending the support for HDR on DP encoders for > > the same. > > > > v2: Limited to non eDP ports on hsw/bdw and removed it for lspcon as > > it is done separately (suggested by Ville) > > > > v3: Added helper and limited eDP restriction to port A (Ville) > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5389 > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 22 +++++++++++++++++++++- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 9e19165fd175..c993d82c7ec9 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -4913,6 +4913,26 @@ bool intel_dp_is_port_edp(struct drm_i915_private > *dev_priv, enum port port) > > return intel_bios_is_port_edp(dev_priv, port); } > > > > +static bool > > +has_gamut_metadata_dip(struct drm_i915_private *dev_priv, > > s/dev_priv/i915/ for modern style Ok, will update > > > + struct intel_dp *intel_dp, enum port port) { > > + if (intel_bios_is_lspcon_present(dev_priv, port)) > > + return false; > > + > > + if (DISPLAY_VER(dev_priv) >= 10 && !IS_GEMINILAKE(dev_priv)) > > + return true; > > DISPLAY_VER >= 11 > > > + > > + if (port == PORT_A && intel_dp_is_edp(intel_dp)) > > The is_edp check is still wrong. Should be just port==A. > Also allows you to drop the intel_dp argument to the function. In the register description for VIDEO_DIP_CTL, (Bspec:7748) BitField: VDIP Enable GMP [BDW, SKL, BXT, KBL, KBLH, GLK, GLV, CFL, WHL, AML, CML, CNL, LKFR] GMP is not supported on transcoder EDP going to DDI A. That's why was checking for eDP. But port A check should be good enough. Will drop it. > > + return false; > > + > > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || > > + IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 9) > > + return true; > > The IS_GLK check is redundant. Yeah will drop it. > > + > > + return false; > > +} > > + > > static void > > intel_dp_add_properties(struct intel_dp *intel_dp, struct > > drm_connector *connector) { @@ -4939,7 +4959,7 @@ > > intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > intel_attach_dp_colorspace_property(connector); > > } > > > > - if (IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 11) > > + if (has_gamut_metadata_dip(dev_priv, intel_dp, port)) > > drm_object_attach_property(&connector->base, > > connector->dev- > >mode_config.hdr_output_metadata_property, > > 0); > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
On Thu, Mar 24, 2022 at 11:58:15AM +0000, Shankar, Uma wrote: > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Thursday, March 24, 2022 5:12 PM > > To: Shankar, Uma <uma.shankar@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [v3] drm/i915/display: Extend DP HDR support to hsw+ > > > > On Thu, Mar 24, 2022 at 04:39:59PM +0530, Uma Shankar wrote: > > > HSW+ platforms are able to send out HDR Metadata SDP DIP > > > packet as GMP. Hence, extending the support for HDR on DP encoders for > > > the same. > > > > > > v2: Limited to non eDP ports on hsw/bdw and removed it for lspcon as > > > it is done separately (suggested by Ville) > > > > > > v3: Added helper and limited eDP restriction to port A (Ville) > > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5389 > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 22 +++++++++++++++++++++- > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 9e19165fd175..c993d82c7ec9 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -4913,6 +4913,26 @@ bool intel_dp_is_port_edp(struct drm_i915_private > > *dev_priv, enum port port) > > > return intel_bios_is_port_edp(dev_priv, port); } > > > > > > +static bool > > > +has_gamut_metadata_dip(struct drm_i915_private *dev_priv, > > > > s/dev_priv/i915/ for modern style > > Ok, will update > > > > > > + struct intel_dp *intel_dp, enum port port) { > > > + if (intel_bios_is_lspcon_present(dev_priv, port)) > > > + return false; > > > + > > > + if (DISPLAY_VER(dev_priv) >= 10 && !IS_GEMINILAKE(dev_priv)) > > > + return true; > > > > DISPLAY_VER >= 11 > > > > > + > > > + if (port == PORT_A && intel_dp_is_edp(intel_dp)) > > > > The is_edp check is still wrong. Should be just port==A. > > Also allows you to drop the intel_dp argument to the function. > > In the register description for VIDEO_DIP_CTL, (Bspec:7748) > BitField: VDIP Enable GMP > [BDW, SKL, BXT, KBL, KBLH, GLK, GLV, CFL, WHL, AML, CML, CNL, LKFR] > GMP is not supported on transcoder EDP going to DDI A. > > That's why was checking for eDP. But port A check should be good enough. Transcoder EDP is hardwired to DDI A. Despite the name it has nothing to do with eDP vs. DP. Having the eDP check would be just wrong because it would then attach the property to DDI A when it is used as an external DP port. And yes, such machines do in fact exist. > Will drop it. > > > > + return false; > > > + > > > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || > > > + IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 9) > > > + return true; > > > > The IS_GLK check is redundant. > > Yeah will drop it. > > > > + > > > + return false; > > > +} > > > + > > > static void > > > intel_dp_add_properties(struct intel_dp *intel_dp, struct > > > drm_connector *connector) { @@ -4939,7 +4959,7 @@ > > > intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > > intel_attach_dp_colorspace_property(connector); > > > } > > > > > > - if (IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 11) > > > + if (has_gamut_metadata_dip(dev_priv, intel_dp, port)) > > > drm_object_attach_property(&connector->base, > > > connector->dev- > > >mode_config.hdr_output_metadata_property, > > > 0); > > > -- > > > 2.25.1 > > > > -- > > Ville Syrjälä > > Intel
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, March 24, 2022 6:30 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [v3] drm/i915/display: Extend DP HDR support to hsw+ > > On Thu, Mar 24, 2022 at 11:58:15AM +0000, Shankar, Uma wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: Thursday, March 24, 2022 5:12 PM > > > To: Shankar, Uma <uma.shankar@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [v3] drm/i915/display: Extend DP HDR support to hsw+ > > > > > > On Thu, Mar 24, 2022 at 04:39:59PM +0530, Uma Shankar wrote: > > > > HSW+ platforms are able to send out HDR Metadata SDP DIP > > > > packet as GMP. Hence, extending the support for HDR on DP encoders > > > > for the same. > > > > > > > > v2: Limited to non eDP ports on hsw/bdw and removed it for lspcon > > > > as it is done separately (suggested by Ville) > > > > > > > > v3: Added helper and limited eDP restriction to port A (Ville) > > > > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5389 > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 22 > > > > +++++++++++++++++++++- > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index 9e19165fd175..c993d82c7ec9 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -4913,6 +4913,26 @@ bool intel_dp_is_port_edp(struct > > > > drm_i915_private > > > *dev_priv, enum port port) > > > > return intel_bios_is_port_edp(dev_priv, port); } > > > > > > > > +static bool > > > > +has_gamut_metadata_dip(struct drm_i915_private *dev_priv, > > > > > > s/dev_priv/i915/ for modern style > > > > Ok, will update > > > > > > > > > + struct intel_dp *intel_dp, enum port port) { > > > > + if (intel_bios_is_lspcon_present(dev_priv, port)) > > > > + return false; > > > > + > > > > + if (DISPLAY_VER(dev_priv) >= 10 && !IS_GEMINILAKE(dev_priv)) > > > > + return true; > > > > > > DISPLAY_VER >= 11 > > > > > > > + > > > > + if (port == PORT_A && intel_dp_is_edp(intel_dp)) > > > > > > The is_edp check is still wrong. Should be just port==A. > > > Also allows you to drop the intel_dp argument to the function. > > > > In the register description for VIDEO_DIP_CTL, (Bspec:7748) > > BitField: VDIP Enable GMP > > [BDW, SKL, BXT, KBL, KBLH, GLK, GLV, CFL, WHL, AML, CML, CNL, LKFR] > > GMP is not supported on transcoder EDP going to DDI A. > > > > That's why was checking for eDP. But port A check should be good enough. > > Transcoder EDP is hardwired to DDI A. Despite the name it has nothing to do with > eDP vs. DP. > > Having the eDP check would be just wrong because it would then attach the property > to DDI A when it is used as an external DP port. And yes, such machines do in fact > exist. Yeah got the point. Thanks Ville for the review and valuable comments. Regards, Uma Shankar > > > Will drop it. > > > > > > + return false; > > > > + > > > > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || > > > > + IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 9) > > > > + return true; > > > > > > The IS_GLK check is redundant. > > > > Yeah will drop it. > > > > > > + > > > > + return false; > > > > +} > > > > + > > > > static void > > > > intel_dp_add_properties(struct intel_dp *intel_dp, struct > > > > drm_connector *connector) { @@ -4939,7 +4959,7 @@ > > > > intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector > *connect > > > > intel_attach_dp_colorspace_property(connector); > > > > } > > > > > > > > - if (IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 11) > > > > + if (has_gamut_metadata_dip(dev_priv, intel_dp, port)) > > > > drm_object_attach_property(&connector->base, > > > > connector->dev- > > > >mode_config.hdr_output_metadata_property, > > > > 0); > > > > -- > > > > 2.25.1 > > > > > > -- > > > Ville Syrjälä > > > Intel > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 9e19165fd175..c993d82c7ec9 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4913,6 +4913,26 @@ bool intel_dp_is_port_edp(struct drm_i915_private *dev_priv, enum port port) return intel_bios_is_port_edp(dev_priv, port); } +static bool +has_gamut_metadata_dip(struct drm_i915_private *dev_priv, + struct intel_dp *intel_dp, enum port port) +{ + if (intel_bios_is_lspcon_present(dev_priv, port)) + return false; + + if (DISPLAY_VER(dev_priv) >= 10 && !IS_GEMINILAKE(dev_priv)) + return true; + + if (port == PORT_A && intel_dp_is_edp(intel_dp)) + return false; + + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) || + IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 9) + return true; + + return false; +} + static void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector) { @@ -4939,7 +4959,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect intel_attach_dp_colorspace_property(connector); } - if (IS_GEMINILAKE(dev_priv) || DISPLAY_VER(dev_priv) >= 11) + if (has_gamut_metadata_dip(dev_priv, intel_dp, port)) drm_object_attach_property(&connector->base, connector->dev->mode_config.hdr_output_metadata_property, 0);
HSW+ platforms are able to send out HDR Metadata SDP DIP packet as GMP. Hence, extending the support for HDR on DP encoders for the same. v2: Limited to non eDP ports on hsw/bdw and removed it for lspcon as it is done separately (suggested by Ville) v3: Added helper and limited eDP restriction to port A (Ville) Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5389 Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)