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>
>
> How about instead of else, move the reg = SPLL_CTL above the 'if'
>
> --
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
