* intel_pre_enable_dp
@ 2013-09-29 23:48 Andi Kleen
2013-09-29 23:53 ` intel_pre_enable_dp Andi Kleen
0 siblings, 1 reply; 3+ messages in thread
From: Andi Kleen @ 2013-09-29 23:48 UTC (permalink / raw)
To: jbarnes; +Cc: intel-gfx
Jesse,
I was playing with a static analyzer and it flagged the following
code of yours.
Is the val = 0 directly after the register read correct and intended?
commit 89b667f86a62a99a7b484a7e1b3f8f7a108a7dee
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Thu Apr 18 14:51:36 2013 -0700
drm/i915: update VLV PLL and DPIO code v11
...
static void intel_pre_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
if (is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev))
ironlake_edp_pll_on(intel_dp);
+
+ if (IS_VALLEYVIEW(dev)) {
+ struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(encoder->base.crtc);
+ int port = vlv_dport_to_channel(dport);
+ int pipe = intel_crtc->pipe;
+ u32 val;
+
+ WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
+
+ val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+ val = 0; <------------- val is directly overriden, previous value is ignored
+ if (pipe)
+ val |= (1<<21);
+ else
+ val &= ~(1<<21);
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: intel_pre_enable_dp
2013-09-29 23:48 intel_pre_enable_dp Andi Kleen
@ 2013-09-29 23:53 ` Andi Kleen
2013-09-30 0:16 ` intel_pre_enable_dp Jesse Barnes
0 siblings, 1 reply; 3+ messages in thread
From: Andi Kleen @ 2013-09-29 23:53 UTC (permalink / raw)
To: Andi Kleen; +Cc: intel-gfx
On Mon, Sep 30, 2013 at 01:48:13AM +0200, Andi Kleen wrote:
>
> Jesse,
>
> I was playing with a static analyzer and it flagged the following
> code of yours.
>
> Is the val = 0 directly after the register read correct and intended?
Also the same pattern is in other places, like vlv_pre_enable_dp.
-Andi
>
> commit 89b667f86a62a99a7b484a7e1b3f8f7a108a7dee
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date: Thu Apr 18 14:51:36 2013 -0700
>
> drm/i915: update VLV PLL and DPIO code v11
> ...
>
> static void intel_pre_enable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>
> if (is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev))
> ironlake_edp_pll_on(intel_dp);
> +
> + if (IS_VALLEYVIEW(dev)) {
> + struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
> + struct intel_crtc *intel_crtc =
> + to_intel_crtc(encoder->base.crtc);
> + int port = vlv_dport_to_channel(dport);
> + int pipe = intel_crtc->pipe;
> + u32 val;
> +
> + WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> +
> + val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
> + val = 0; <------------- val is directly overriden, previous value is ignored
> + if (pipe)
> + val |= (1<<21);
> + else
> + val &= ~(1<<21);
>
>
>
>
>
> --
> ak@linux.intel.com -- Speaking for myself only.
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: intel_pre_enable_dp
2013-09-29 23:53 ` intel_pre_enable_dp Andi Kleen
@ 2013-09-30 0:16 ` Jesse Barnes
0 siblings, 0 replies; 3+ messages in thread
From: Jesse Barnes @ 2013-09-30 0:16 UTC (permalink / raw)
To: Andi Kleen; +Cc: intel-gfx
On Mon, 30 Sep 2013 01:53:47 +0200
Andi Kleen <andi@firstfloor.org> wrote:
> On Mon, Sep 30, 2013 at 01:48:13AM +0200, Andi Kleen wrote:
> >
> > Jesse,
> >
> > I was playing with a static analyzer and it flagged the following
> > code of yours.
> >
> > Is the val = 0 directly after the register read correct and intended?
>
> Also the same pattern is in other places, like vlv_pre_enable_dp.
Hm definitely looks suspect... I'll check it out. On the plus side
that code is actually working in practice, so maybe the zeroing isn't
too harmful. :)
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-30 0:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-29 23:48 intel_pre_enable_dp Andi Kleen
2013-09-29 23:53 ` intel_pre_enable_dp Andi Kleen
2013-09-30 0:16 ` intel_pre_enable_dp Jesse Barnes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox