Date: Wed, 13 May 2015 20:21:18 -0700
From: Matt Roper <matthew.d.roper@intel.com>
To: Thomas Gummerer <t.gummerer@gmail.com>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, David Airlie <airlied@linux.ie>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Daniel Vetter <daniel.vetter@intel.com>, jan@gondor.com,
        Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
        Michael Leuchtenburg <michael@slashhome.org>
Subject: Re: [BUG/REGRESSION] Screen flickering
Message-ID: <20150514032118.GD25648@intel.com>
References: <87pp65efmo.fsf@hank.tgummerer.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87pp65efmo.fsf@hank.tgummerer.com>
User-Agent: Mutt/1.5.20 (2009-06-14)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2573
Lines: 63

On Tue, May 12, 2015 at 09:54:07PM +0200, Thomas Gummerer wrote:
> 
> Hi,
> 
> I noticed that on my machine the screen starts to flicker after I
> suspend and resume my machine, on the main laptop display if an external
> display is attached with kernel v4.1-rc1.  I tracked the regression down
> to commit c9f038a1a592 ("drm/i915: Don't assume primary & cursor are
> always on for wm calculation (v4)"), and sent a patch that fixes that
> behavior at [1] about two weeks ago, although I'm not sure it's the
> right thing to do, as I'm not very familiar with the code.  The same bug
> still exists in vv4.1-rc3.
> 
> Jan Niehusmann confirmed the behavior, but there has been no further
> discussion on the topic.  I also forgot to cc the people that were
> involved in the patch that caused the regression (sorry about that).
> 
> Is there anything else that I can do to help fixing this issue?
> 
> Thanks,
> Thomas
> 
> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-April/065494.html

Sorry, I missed your patch when you first sent it.  That type of fix
looks like an okay workaround while we do a more in-depth rework of the
watermark system.  However I think your patch could cause a crash if we
disable the primary plane via the universal plane interface; if we do
that, p->pri.bytes_per_pixel is set to 0, but since we're now pretending
the primary plane is always enabled, ilk_wm_fbc() can eventually get
called and use that 0 in the denominator of a division operation.

If you just squash the following change into your patch, I think it should be
safe:

        diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
        index eb97cbe..99fa8ee 100644
        --- a/drivers/gpu/drm/i915/intel_pm.c
        +++ b/drivers/gpu/drm/i915/intel_pm.c
        @@ -2076,7 +2076,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
                        p->pri.bytes_per_pixel =
                                crtc->primary->state->fb->bits_per_pixel / 8;
                else
        -               p->pri.bytes_per_pixel = 0;
        +               p->pri.bytes_per_pixel = 4;
         
                p->cur.bytes_per_pixel = 4;
                /*


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/