From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Add a bit of locking to intel_dp_hpd_pulse() Date: Fri, 1 Aug 2014 14:55:22 +0300 Message-ID: <20140801115522.GX4193@intel.com> References: <1406734764-19234-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 689C289AE6 for ; Fri, 1 Aug 2014 04:55:45 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Dave Airlie Cc: Dave Airlie , "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Thu, Jul 31, 2014 at 08:59:08PM +1000, Dave Airlie wrote: > On 31 July 2014 17:37, Daniel Vetter wrote: > > On Thu, Jul 31, 2014 at 1:49 AM, Dave Airlie wrote: > >> Daniel, the only way intel_dp->is_mst can get reset is inside this pat= h. > > > > Ok, so that one should be safe. Then I guess we can just push the > > locking down into the respective non-mst leafs (since atm we do > > link-retraining without any locking, which isn't good). And it needs > > to be dev->mode_config.mutex, not connection mutex. Why that? We can't be doing a modeset w/o connection_mutex so that seems like it should be enough. Well, there's also dpms which leaves the crtc<->encoder<->connector links intact but that too takes connection_mutex currently. > = > I'd like to know why we do link training at this point though as well, > adding locking is required of course, I was just going to wrap the > short irq call to the link status check with the lock, but I think it > should be possible to push it down further, I don't really know why the sink generates the hpd when we turn off the port, but that doesn't really matter I think. We need to be prepared for hpds at any time. intel_dp_check_link_status() just checks if there's a crtc, which there is (either the old one or the new one, depending on how far along the modeset path we are I guess), and then it just checks drm_dp_channel_eq_ok() which says false since the link was turned off, and then it proceeds to retrain the link. Maybe it should also check crtc->active? Though that itself won't = eliminate the problem unless the locking gets fixed somehow. > = > I'm not sure how vague the spec is on what should happen on HPDs, but > if we drop the port clock we obviously will lose the link, but we > should also know not to be retraining it at that point anyways. > = > Dave. -- = Ville Syrj=E4l=E4 Intel OTC