From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v5 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read() Date: Thu, 14 Apr 2016 20:01:43 +0300 Message-ID: <20160414170143.GS4329@intel.com> References: <87d1pvt9ka.fsf@intel.com> <1460559513-32280-1-git-send-email-cpaul@redhat.com> <1460559513-32280-4-git-send-email-cpaul@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1460559513-32280-4-git-send-email-cpaul@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lyude Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, open list List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBBcHIgMTMsIDIwMTYgYXQgMTA6NTg6MzJBTSAtMDQwMCwgTHl1ZGUgd3JvdGU6Cj4g VGhpcyBpcyBwYXJ0IG9mIGEgcGF0Y2ggc2VyaWVzIHRvIG1pZ3JhdGUgYWxsIG9mIHRoZSB3b3Jr YXJvdW5kcyBmb3IKPiBjb21tb25seSBzZWVuIGJlaGF2aW9yIGZyb20gYmFkIHNpbmtzIGluIGlu dGVsX2RwX2RwY2RfcmVhZF93YWtlKCkgdG8gZHJtJ3MKPiBEUCBoZWxwZXIuCj4gCj4gU29tZSBz aW5rcyB3aWxsIGp1c3QgcmV0dXJuIGdhcmJhZ2UgZm9yIHRoZSBmaXJzdCBhdXggdHJhbmFjdGlv biB0aGV5Cj4gcmVjZWl2ZSB3aGVuIGNvbWluZyBvdXQgb2Ygc2xlZXAgbW9kZSwgc28gd2UgbmVl ZCB0byBwZXJmb3JtIGFuIGFkZGl0aW9uYWwKPiByZWFkIGJlZm9yZSB0aGUgYWN0dWFsIHJlYWQg dG8gd29ya2Fyb3VuZCB0aGlzLgo+IAo+IAkJCSAgICBDaGFuZ2VzIHNpbmNlIHY1Cj4gLSBJZiB0 aGUgdGhyb3dhd2F5IHJlYWQgaW4gZHJtX2RwX2RwY2RfcmVhZCgpIGZhaWxzLCByZXR1cm4gdGhl IGVycm9yCj4gICBmcm9tIHRoYXQgaW5zdGVhZCBvZiBjb250aW51aW5nLiBUaGlzIGZvbGxvd3Mg dGhlIHNhbWUgbG9naWMgd2UgZG8gaW4KPiAgIGRybV9kcF9kcGNkX2FjY2VzcygpIChlLmcuIHRo ZSBlcnJvciBmcm9tIHRoZSBmaXJzdCB0cmFuc2FjdGlvbiBtYXkKPiAgIGRpZmZlciBmcm9tIHRo ZSBlcnJvcnMgdGhhdCBwcm9jZWVkaW5nIGF0dGVtcHRzIG1pZ2h0IHJldHVybikuCj4gCj4gU2ln bmVkLW9mZi1ieTogTHl1ZGUgPGNwYXVsQHJlZGhhdC5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1 L2RybS9kcm1fZHBfaGVscGVyLmMgfCAxMiArKysrKysrKysrKysKPiAgMSBmaWxlIGNoYW5nZWQs IDEyIGluc2VydGlvbnMoKykKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9k cF9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZHBfaGVscGVyLmMKPiBpbmRleCA1NDBj M2U0Li5hYTgwNzAyIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fZHBfaGVscGVy LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2RwX2hlbHBlci5jCj4gQEAgLTI0OCw2ICsy NDgsMTggQEAgdW5sb2NrOgo+ICBzc2l6ZV90IGRybV9kcF9kcGNkX3JlYWQoc3RydWN0IGRybV9k cF9hdXggKmF1eCwgdW5zaWduZWQgaW50IG9mZnNldCwKPiAgCQkJIHZvaWQgKmJ1ZmZlciwgc2l6 ZV90IHNpemUpCj4gIHsKPiArCWludCByZXQ7Cj4gKwo+ICsJLyoKPiArCSAqIFNvbWV0aW1lcyB3 ZSBqdXN0IGdldCB0aGUgc2FtZSBpbmNvcnJlY3QgYnl0ZSByZXBlYXRlZCBvdmVyIHRoZQo+ICsJ ICogZW50aXJlIGJ1ZmZlci4gRG9pbmcgb25lIHRocm93IGF3YXkgcmVhZCBpbml0aWFsbHkgc2Vl bXMgdG8gInNvbHZlIgo+ICsJICogaXQuCj4gKwkgKi8KCldlIG1pZ2h0IHdhbnQgdG8gaW1wcm92 ZSB0aGUgY29tbWVudCBhIGJpdCwgbm93IHRoYXQgSSBhbmFseXplZCB0aGUKcHJvYmxlbSBpbiBt b3JlIGRldGFpbC4gU28gcGVyaGFwcyBzb21ldGhpbmcgbGlrZToKCi8qCiAqIEhQIFpSMjR3IGNv cnJ1cHRzIHRoZSBmaXJzdCBEUENEIGFjY2VzcyBhZnRlciBlbnRlcmluZyBwb3dlciBzYXZlCiAq IG1vZGUuIEVnLiBvbiBhIHJlYWQsIHRoZSBlbnRpcmUgYnVmZmVyIHdpbGwgYmUgZmlsbGVkIHdp dGggdGhlIHNhbWUKICogYnl0ZS4gRG8gYSB0aHJvdyBhd2F5IHJlYWQgdG8gYXZvaWQgY29ycnVw dGluZyBhbnl0aGluZyB3ZSBjYXJlCiAqIGFib3V0LiBBZnRlcndhcmRzIHRoaW5ncyB3aWxsIHdv cmsgY29ycmVjdGx5IHVudGlsIHRoZSBtb25pdG9yCiAqIGdldHMgd29rZW4gdXAgYW5kIHN1YnNl cXVlbnRseSByZS1lbnRlcnMgcG93ZXIgc2F2ZSBtb2RlLgogKgogKiBUaGUgdXNlciBwcmVzc2lu ZyBhbnkgYnV0dG9uIG9uIHRoZSBtb25pdG9yIGlzIGVub3VnaCB0byB3YWtlIGl0CiAqIHVwLCBz byB0aGVyZSBpcyBubyBwYXJ0aWN1bGFybHkgZ29vZCBwbGFjZSB0byBkbyB0aGUgd29ya2Fyb3Vu ZC4KICogV2UganVzdCBoYXZlIHRvIGRvIGl0IGJlZm9yZSBhbnkgRFBDRCBhY2Nlc3MgYW5kIGhv cGUgdGhhdCB0aGUKICogbW9uaXRvciBkb2Vzbid0IHBvd2VyIGRvd24gZXhhY3RseSBhZnRlciB0 aGUgdGhyb3cgYXdheSByZWFkLgogKi8KCkknbSB0aGlua2luZyB3ZSBzaG91bGQgcHJvYmFibHkg YWxzbyBkbyBpdCBmb3Igd3JpdGVzLCBzaW5jZSB0aG9zZQpzZWVtIHRvIGJlIGFmZmVjdGVkIGFz IHdlbGwuCgo+ICsJcmV0ID0gZHJtX2RwX2RwY2RfYWNjZXNzKGF1eCwgRFBfQVVYX05BVElWRV9S RUFELCBEUF9EUENEX1JFViwgYnVmZmVyLAo+ICsJCQkJIDEpOwo+ICsJaWYgKHJldCAhPSAxKQo+ ICsJCXJldHVybiByZXQ7Cj4gKwo+ICAJcmV0dXJuIGRybV9kcF9kcGNkX2FjY2VzcyhhdXgsIERQ X0FVWF9OQVRJVkVfUkVBRCwgb2Zmc2V0LCBidWZmZXIsCj4gIAkJCQkgIHNpemUpOwo+ICB9Cj4g LS0gCj4gMi41LjUKCi0tIApWaWxsZSBTeXJqw6Rsw6QKSW50ZWwgT1RDCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422676AbcDNRBu (ORCPT ); Thu, 14 Apr 2016 13:01:50 -0400 Received: from mga14.intel.com ([192.55.52.115]:30462 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752324AbcDNRBt (ORCPT ); Thu, 14 Apr 2016 13:01:49 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,485,1455004800"; d="scan'208";a="945280030" Date: Thu, 14 Apr 2016 20:01:43 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Lyude Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, Jani Nikula , David Airlie , open list Subject: Re: [PATCH v5 3/4] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read() Message-ID: <20160414170143.GS4329@intel.com> References: <87d1pvt9ka.fsf@intel.com> <1460559513-32280-1-git-send-email-cpaul@redhat.com> <1460559513-32280-4-git-send-email-cpaul@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1460559513-32280-4-git-send-email-cpaul@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 13, 2016 at 10:58:32AM -0400, Lyude wrote: > This is part of a patch series to migrate all of the workarounds for > commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's > DP helper. > > Some sinks will just return garbage for the first aux tranaction they > receive when coming out of sleep mode, so we need to perform an additional > read before the actual read to workaround this. > > Changes since v5 > - If the throwaway read in drm_dp_dpcd_read() fails, return the error > from that instead of continuing. This follows the same logic we do in > drm_dp_dpcd_access() (e.g. the error from the first transaction may > differ from the errors that proceeding attempts might return). > > Signed-off-by: Lyude > --- > drivers/gpu/drm/drm_dp_helper.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 540c3e4..aa80702 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -248,6 +248,18 @@ unlock: > ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, > void *buffer, size_t size) > { > + int ret; > + > + /* > + * Sometimes we just get the same incorrect byte repeated over the > + * entire buffer. Doing one throw away read initially seems to "solve" > + * it. > + */ We might want to improve the comment a bit, now that I analyzed the problem in more detail. So perhaps something like: /* * HP ZR24w corrupts the first DPCD access after entering power save * mode. Eg. on a read, the entire buffer will be filled with the same * byte. Do a throw away read to avoid corrupting anything we care * about. Afterwards things will work correctly until the monitor * gets woken up and subsequently re-enters power save mode. * * The user pressing any button on the monitor is enough to wake it * up, so there is no particularly good place to do the workaround. * We just have to do it before any DPCD access and hope that the * monitor doesn't power down exactly after the throw away read. */ I'm thinking we should probably also do it for writes, since those seem to be affected as well. > + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer, > + 1); > + if (ret != 1) > + return ret; > + > return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer, > size); > } > -- > 2.5.5 -- Ville Syrjälä Intel OTC