diff mbox

[08/15] drm/i915: Reject >9 ddi translation entried if port != A/E on SKL

Message ID 1449597590-6971-9-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Dec. 8, 2015, 5:59 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Only DDI A and E support 10 translation entries in DP mode. For the
other ports the tenth entry is reserved for HDMI..

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Daniel Vetter Dec. 10, 2015, 1:30 p.m. UTC | #1 
On Tue, Dec 08, 2015 at 07:59:43PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Only DDI A and E support 10 translation entries in DP mode. For the
> other ports the tenth entry is reserved for HDMI..
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 838cbbe33517..152c813cc43e 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -443,6 +443,10 @@ static void intel_prepare_ddi_buffers(struct drm_i915_private *dev_priv,
>  		if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level ||
>  		    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
>  			iboost_bit = 1<<31;
> +
> +		if (WARN_ON(port != PORT_A &&
> +			    port != PORT_E && n_edp_entries > 9))
> +			n_edp_entries = 9;

Imo WARN_ON just here is enough, set_iboost can only be hit if we pass
here. With that bikeshed Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  	} else if (IS_BROADWELL(dev_priv)) {
>  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_fdi) != 9);
>  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_dp) != 9);
> @@ -2099,6 +2103,11 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
>  			iboost = dp_iboost;
>  		} else {
>  			ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
> +
> +			if (WARN_ON(port != PORT_A &&
> +				    port != PORT_E && n_entries > 9))
> +				n_entries = 9;
> +
>  			iboost = ddi_translations[level].i_boost;
>  		}
>  	} else if (type == INTEL_OUTPUT_HDMI) {
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Dec. 10, 2015, 1:42 p.m. UTC | #2 
On Thu, Dec 10, 2015 at 02:30:34PM +0100, Daniel Vetter wrote:
> On Tue, Dec 08, 2015 at 07:59:43PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Only DDI A and E support 10 translation entries in DP mode. For the
> > other ports the tenth entry is reserved for HDMI..
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 838cbbe33517..152c813cc43e 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -443,6 +443,10 @@ static void intel_prepare_ddi_buffers(struct drm_i915_private *dev_priv,
> >  		if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level ||
> >  		    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
> >  			iboost_bit = 1<<31;
> > +
> > +		if (WARN_ON(port != PORT_A &&
> > +			    port != PORT_E && n_edp_entries > 9))
> > +			n_edp_entries = 9;
> 
> Imo WARN_ON just here is enough, set_iboost can only be hit if we pass
> here. With that bikeshed Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

We would then access some unknown registers and memory.

> 
> >  	} else if (IS_BROADWELL(dev_priv)) {
> >  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_fdi) != 9);
> >  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_dp) != 9);
> > @@ -2099,6 +2103,11 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
> >  			iboost = dp_iboost;
> >  		} else {
> >  			ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
> > +
> > +			if (WARN_ON(port != PORT_A &&
> > +				    port != PORT_E && n_entries > 9))
> > +				n_entries = 9;
> > +
> >  			iboost = ddi_translations[level].i_boost;
> >  		}
> >  	} else if (type == INTEL_OUTPUT_HDMI) {
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Dec. 10, 2015, 2:09 p.m. UTC | #3 
On Thu, Dec 10, 2015 at 03:42:15PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 10, 2015 at 02:30:34PM +0100, Daniel Vetter wrote:
> > On Tue, Dec 08, 2015 at 07:59:43PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Only DDI A and E support 10 translation entries in DP mode. For the
> > > other ports the tenth entry is reserved for HDMI..
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 838cbbe33517..152c813cc43e 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -443,6 +443,10 @@ static void intel_prepare_ddi_buffers(struct drm_i915_private *dev_priv,
> > >  		if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level ||
> > >  		    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
> > >  			iboost_bit = 1<<31;
> > > +
> > > +		if (WARN_ON(port != PORT_A &&
> > > +			    port != PORT_E && n_edp_entries > 9))
> > > +			n_edp_entries = 9;
> > 
> > Imo WARN_ON just here is enough, set_iboost can only be hit if we pass
> > here. With that bikeshed Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> We would then access some unknown registers and memory.

Well tbh I'm not too concerning about trampling over random registers.
Generally the chip survives, so one WARN to scream at you is enough.
Personally I wouldn't even bother with the if () return.

Anyway this is bikeshed, so if you feel this is useful you can extend the
r-b to the entire patch.
-Daniel

> 
> > 
> > >  	} else if (IS_BROADWELL(dev_priv)) {
> > >  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_fdi) != 9);
> > >  		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_dp) != 9);
> > > @@ -2099,6 +2103,11 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
> > >  			iboost = dp_iboost;
> > >  		} else {
> > >  			ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
> > > +
> > > +			if (WARN_ON(port != PORT_A &&
> > > +				    port != PORT_E && n_entries > 9))
> > > +				n_entries = 9;
> > > +
> > >  			iboost = ddi_translations[level].i_boost;
> > >  		}
> > >  	} else if (type == INTEL_OUTPUT_HDMI) {
> > > -- 
> > > 2.4.10
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 838cbbe33517..152c813cc43e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -443,6 +443,10 @@  static void intel_prepare_ddi_buffers(struct drm_i915_private *dev_priv,
 		if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level ||
 		    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
 			iboost_bit = 1<<31;
+
+		if (WARN_ON(port != PORT_A &&
+			    port != PORT_E && n_edp_entries > 9))
+			n_edp_entries = 9;
 	} else if (IS_BROADWELL(dev_priv)) {
 		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_fdi) != 9);
 		BUILD_BUG_ON(ARRAY_SIZE(bdw_ddi_translations_dp) != 9);
@@ -2099,6 +2103,11 @@  static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 			iboost = dp_iboost;
 		} else {
 			ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
+
+			if (WARN_ON(port != PORT_A &&
+				    port != PORT_E && n_entries > 9))
+				n_entries = 9;
+
 			iboost = ddi_translations[level].i_boost;
 		}
 	} else if (type == INTEL_OUTPUT_HDMI) {