From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: DPCD/AUX locking Date: Wed, 7 May 2014 10:01:33 +0200 Message-ID: <20140507080132.GA2842@ulmo> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0431359706==" Return-path: Received: from mail-ee0-f42.google.com (mail-ee0-f42.google.com [74.125.83.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 1A5DB6EC66 for ; Wed, 7 May 2014 01:03:25 -0700 (PDT) Received: by mail-ee0-f42.google.com with SMTP id d49so427740eek.1 for ; Wed, 07 May 2014 01:03:25 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Dave Airlie Cc: Daniel Vetter , dri-devel List-Id: dri-devel@lists.freedesktop.org --===============0431359706== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="lrZ03NoBR/3+SXJZ" Content-Disposition: inline --lrZ03NoBR/3+SXJZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 07, 2014 at 05:12:54PM +1000, Dave Airlie wrote: > On 7 May 2014 15:26, Ben Skeggs wrote: > > On Wed, May 7, 2014 at 9:20 AM, Dave Airlie wrote: > >> So now I've been playing with MST I think get the feeling I might need > >> some explicit locking on the AUX channel, I think at the moment the > >> mode_config mutex implicitly defends the aux channel as the only real > >> paths into it are > >> > >> a) from userspace connector probing, > >> b) from HPD irqs, > >> > >> currently both of these on i915 at least take mode config, > >> > >> however with MST I can't use mode_config for this, so I'm wondering if > >> I should be adding some explicit locking in the helpers or make it the > >> drivers problem to lock around helper access? > > Without yet being clear on what you're locking against exactly, my > > vote would be on making this the driver's problem. We (should be, but > > don't yet) need to take locks around AUX access anyway as the pads are > > shared between aux/ddc channels in a lot of cases. >=20 > locking against concurrent access, >=20 > currently if I get a HPD irq that requires reading the DPCD status, > and I get a connector detect from userspace that reads i2c over aux > they would collide, >=20 > at the moment mode config lock seems to stop that in i915 but taking mode= config > for the DP HPD irq is very wrong for MST. I think that if concurrent access is all that you're worried about, then having a lock in each driver's drm_dp_aux implementation should work. If we end up doing that for every driver then it probably makes sense to move it into drm_dp_aux directly and handle locking within the helpers. That has the disadvantage that somebody could for some reason decide to call into the driver's ->transfer function directly, in which case that code will have to make sure to do proper locking itself. Depending on what you want to do I guess it would make sense to introduce two levels of locking. For example the DPCD and I2C-over-AUX helpers have retry logic built in, so it might make sense to lock around the loops as well in order for concurrent accesses not to be interleaved with the subsequent retries. If the locking happens within the driver's ->transfer implementation it would still be possible for other DPCD accesses to happen in between retries. I'm not sure if that's actually a problem, but I can imagine that it could mess up the whole retry logic. Thierry --lrZ03NoBR/3+SXJZ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTaehcAAoJEN0jrNd/PrOh2bwP/iBMiwOCkR798qRKxP210w+m u//lzIBS2gM53fXvict+8tO+RBpXAxhRrg6OzLtE1O8khpNTvz1KJvrTteWvfGVW iEoh4aTJ63Qgb6kaPagTiG+SydyWBwF95I6NfKkfz955oIEKm08wktzXSRx5TEQt CEioapk0AmJ6f5idHvlaeMWuVEz2YqILI2yMKalqrQfvLGxg68Y09+5nNETXhQ7u VNwc/JMiGeDld8xLrFXHe7KfGlMbVLUKUCDVPiFAs0cIv5HKzBI96V6zk9HIIl/Z ZOqmy+0K6L1kUcp2bqfjFsntFwZJYR0vjwVfzCQLkYMdMzKqvJhJtoGk9aBXhShx 3QFnBeMxBQ3JpZpLToZEKiselSqK1m27NWxhjmlB2L93wSuAdGJs4JrVDKCDx1am CIK6l8GvIQxno9M/N6PAcyijLArZOWaDSCNL3+GsVMj645p7Jvheur1EaAhZ8xCE 6Q+kUtWxnRKjQzTk95PiBjm9yuXuRF0XfLc0sAw9DfbJSYQgXhhkxfZDQzdaYAsr t5wFNP0cp2hKu+MUQjLaknpxGsO2KCStm+eRxB/ClLVd/pbJOWhjix0dE0xcG+KM sHT3nGwpNk8X6ZnlfkGM+3Xpb13Vepvtj1ZZiGcB4Nyfq6rKVMYt/E+fwfCp1KT7 TAa1H1hnT3q43+Q6+p2/ =Clbs -----END PGP SIGNATURE----- --lrZ03NoBR/3+SXJZ-- --===============0431359706== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0431359706==--