* [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ @ 2022-03-24 11:09 Uma Shankar 2022-03-24 11:42 ` Ville Syrjälä 0 siblings, 1 reply; 5+ messages in thread From: Uma Shankar @ 2022-03-24 11:09 UTC (permalink / raw) To: intel-gfx 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, + 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); -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ 2022-03-24 11:09 [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ Uma Shankar @ 2022-03-24 11:42 ` Ville Syrjälä 2022-03-24 11:58 ` Shankar, Uma 0 siblings, 1 reply; 5+ messages in thread From: Ville Syrjälä @ 2022-03-24 11:42 UTC (permalink / raw) To: Uma Shankar; +Cc: intel-gfx 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 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ 2022-03-24 11:42 ` Ville Syrjälä @ 2022-03-24 11:58 ` Shankar, Uma 2022-03-24 12:59 ` Ville Syrjälä 0 siblings, 1 reply; 5+ messages in thread From: Shankar, Uma @ 2022-03-24 11:58 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org > -----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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ 2022-03-24 11:58 ` Shankar, Uma @ 2022-03-24 12:59 ` Ville Syrjälä 2022-03-24 13:19 ` Shankar, Uma 0 siblings, 1 reply; 5+ messages in thread From: Ville Syrjälä @ 2022-03-24 12:59 UTC (permalink / raw) To: Shankar, Uma; +Cc: intel-gfx@lists.freedesktop.org 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 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ 2022-03-24 12:59 ` Ville Syrjälä @ 2022-03-24 13:19 ` Shankar, Uma 0 siblings, 0 replies; 5+ messages in thread From: Shankar, Uma @ 2022-03-24 13:19 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org > -----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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-24 13:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-24 11:09 [Intel-gfx] [v3] drm/i915/display: Extend DP HDR support to hsw+ Uma Shankar 2022-03-24 11:42 ` Ville Syrjälä 2022-03-24 11:58 ` Shankar, Uma 2022-03-24 12:59 ` Ville Syrjälä 2022-03-24 13:19 ` Shankar, Uma
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox