From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-owner@vger.kernel.org>
Received: from mga03.intel.com ([134.134.136.65]:49082 "EHLO mga03.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753861AbeDITOb (ORCPT <rfc822;stable@vger.kernel.org>);
        Mon, 9 Apr 2018 15:14:31 -0400
Date: Mon, 9 Apr 2018 12:14:32 -0700
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Souza, Jose" <jose.souza@intel.com>,
        "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>,
        "R, Durgadoss" <durgadoss.r@intel.com>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/psr: Chase psr.enabled only under
 the psr.lock
Message-ID: <20180409191432.GN8964@intel.com>
References: <20180405114915.29609-1-chris@chris-wilson.co.uk>
 <7e7b4336aefd6e6cf993150c1a83d214c161a210.camel@intel.com>
 <20180406221816.GG8964@intel.com>
 <152309192577.26482.17726023366871782940@mail.alporthouse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <152309192577.26482.17726023366871782940@mail.alporthouse.com>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>

On Sat, Apr 07, 2018 at 10:05:25AM +0100, Chris Wilson wrote:
> Quoting Rodrigo Vivi (2018-04-06 23:18:16)
> > On Fri, Apr 06, 2018 at 11:12:27AM -0700, Souza, Jose wrote:
> > > On Thu, 2018-04-05 at 12:49 +0100, Chris Wilson wrote:
> > > > +           struct drm_crtc *crtc =
> > > > +                   dp_to_dig_port(intel_dp)->base.base.crtc;
> > 
> > I'm afraid that the issue is this pointer here. So this will only mask
> > the issue.
> > 
> > Should we maybe stash the pipe? :/
> 
> It's not that bad. pipe cannot change until after psr_disable is called,
> right? And psr_disable ensures that this worker is flushed. The current
> problem is just the coordination of cancelling the worker, where we may
> set psr.enabled to NULL right before the worker grabs it and
> dereferences it.
> 
> So if we lock until we have the pipe, we know that dereference chain is
> valid, and we know that psr_disable() cannot complete until we complete
> the wait. So the pipe remains valid until we return (so long as the pipe
> exists when we start).

hmm... it makes sense and I have no better suggestion actually.
So, as long it really fixes the regression we introduced:

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx