Re: [PATCH] drm/i915: Protect against wrong reg offset and warn.

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Ter, 2018-09-11 às 14:26 -0700, Rodrigo Vivi escreveu:
> On Tue, Sep 11, 2018 at 01:39:53PM -0700, Paulo Zanoni wrote:
> > Em Dom, 2018-09-02 às 22:15 -0700, Rodrigo Vivi escreveu:
> > > In case we forget to change intel_port_is_tc
> > > we would be trying to access registers with port == -1,
> > > i.e. PORT_TC_NONE, what would be wrong offset.
> > > 
> > 
> > There are some many things that would break if intel_port_is_tc()
> > is
> > incorrect that we would have to add a few dozen more WARNs. Why add
> > this specific check at this specific case and not all other checks
> > in
> > other possible cases that could break?
> 
> because static analyzer tools just bother to inform us about this
> case here...
> 
> but I can mark that as intentional/false-positive/impossible/don't-
> bother-anymore
> and just move one if you think this is really un impossible
> possibility

It's not impossible: every future platform that has TC has the
potential to hit it until someone fixes it. So there is some value to
the WARN. I was just wondering why add the code specifically here,
maybe this is not the best place, but it is.

Perhaps breaking intel_port_is_tc by purpose on ICL and trying to boot
it would tell us the appropriate places to add WARNs and return :).

> > 
> > 
> > > So let's just add a protection and warn here.
> > > 
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 436c22de33b6..6a93fa9bbbd9 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4886,6 +4886,9 @@ static bool icl_tc_port_connected(struct
> > > drm_i915_private *dev_priv,
> > >  	bool is_legacy, is_typec, is_tbt;
> > >  	u32 dpsp;
> > >  
> > > +	if (WARN_ON(tc_port == PORT_TC_NONE))
> > > +		return false;
> > > +
> > >  	is_legacy = I915_READ(SDEISR) &
> > > SDE_TC_HOTPLUG_ICP(tc_port);
> > >  
> > >  	/*
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx