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

[Daniel Vetter <daniel@ffwll.ch>]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx