On Fri, Mar 22, 2013 at 06:14:25PM -0300, Paulo Zanoni wrote:
> Hi
>
> 2013/3/8 Ben Widawsky <ben at bwidawsk.net>:
> > On Thu, Mar 07, 2013 at 03:30:25PM +0000, Damien Lespiau wrote:
> >> Our static analysis tool noticed that 'reg' could be used uninitialized if
> >> we are trying to get a PLL to drive VGA and SPLL is already in use
> >> (plls->spll_refcoung != 0).
>
> This was also reported by a human a few weeks ago and went to my TODO
> list. Thanks for clearing my TODO list :)
>
> >>
> >> In the (error) case above, let's return false to the caller and emit an
> >> error.
> >>
> >> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_ddi.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index a1505e3..0eab200 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -898,6 +898,9 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock)
> >> plls->spll_refcount++;
> >> reg = SPLL_CTL;
> >> intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
> >> + } else {
> >> + DRM_ERROR("SPLL already in use\n");
> >> + return false;
> >> }
> >>
> >> WARN(I915_READ(reg) & SPLL_PLL_ENABLE,
> >
> > I know very little about this code, but the warning still seems valuable
> > to me.
>
> The WARN will still be hit in case no one is using SPLL but it's
> enabled. It's a different problem.
>
> The code added by Damien is correct, but it should never happen with
> the current code because only the analog encoder tries to use SPLL and
> there's only 1 analog encoder. But we can always introduce bugs to
> trigger these errors :)
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
