From mboxrd@z Thu Jan 1 00:00:00 1970 From: jani.nikula@linux.intel.com (Jani Nikula) Date: Thu, 09 Nov 2017 11:49:56 +0200 Subject: [PATCH] drm/bridge: dw-hdmi: fix EDID parsing In-Reply-To: <20171109093122.GA12318@n2100.armlinux.org.uk> References: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> <20171109093122.GA12318@n2100.armlinux.org.uk> Message-ID: <87inejaiij.fsf@intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, 09 Nov 2017, Russell King - ARM Linux wrote: > On Thu, Nov 09, 2017 at 09:23:18AM +0100, Daniel Vetter wrote: >> On Tue, Nov 07, 2017 at 11:27:21AM +0000, Russell King wrote: >> > Parsing the EDID for HDMI and audio information in the get_modes() >> > callback is incorrect - this only parses the EDID read from the >> > connector, not any override or firmware provided EDID. >> > >> > The correct place to parse the EDID for these parameters is the >> > fill_modes() callback, after we've called the helper. Move the parsing >> > there. This caused problems for Lu?s Mendes. >> > >> > Cc: >> > Reported-by: Lu?s Mendes >> > Tested-by: Lu?s Mendes >> > Signed-off-by: Russell King >> > --- >> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 28 ++++++++++++++++++++++++---- >> > 1 file changed, 24 insertions(+), 4 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> > index 9fe407f49986..2516a1c18a10 100644 >> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> > @@ -1905,10 +1905,7 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) >> > dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", >> > edid->width_cm, edid->height_cm); >> > >> > - hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); >> > - hdmi->sink_has_audio = drm_detect_monitor_audio(edid); >> > drm_mode_connector_update_edid_property(connector, edid); >> > - cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); >> > ret = drm_add_edid_modes(connector, edid); >> > /* Store the ELD */ >> > drm_edid_to_eld(connector, edid); >> > @@ -1920,6 +1917,29 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) >> > return ret; >> > } >> > >> > +static int dw_hdmi_connector_fill_modes(struct drm_connector *connector, >> > + uint32_t maxX, uint32_t maxY) >> > +{ >> > + struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, >> > + connector); >> > + int ret; >> > + >> > + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); >> > + >> > + if (connector->edid_blob_ptr) { >> > + struct edid *edid = (void *)connector->edid_blob_ptr->data; >> > + >> > + hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); >> > + hdmi->sink_has_audio = drm_detect_monitor_audio(edid); >> > + cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); >> > + } else { >> > + hdmi->sink_is_hdmi = false; >> > + hdmi->sink_has_audio = false; >> > + } >> > + >> > + return ret; >> > +} >> > + >> > static void dw_hdmi_connector_force(struct drm_connector *connector) >> > { >> > struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, >> > @@ -1933,7 +1953,7 @@ static void dw_hdmi_connector_force(struct drm_connector *connector) >> > } >> > >> > static const struct drm_connector_funcs dw_hdmi_connector_funcs = { >> > - .fill_modes = drm_helper_probe_single_connector_modes, >> > + .fill_modes = dw_hdmi_connector_fill_modes, >> >> Papering over helper functions shouldn't be necessary, except the helper >> functions not handling the override edid is a known issue. Jani Nikula is >> working on a proper fix, please coordinate with him. > > So, what you're basically saying is that fixing real bugs that affect > users is not something that DRM people want. That's fine, I'll ignore > people who come to me for help with DRM bugs in future then because > it's obviously a dead loss. We may already have fixed the bug in drm-next at the proper layer. Please see my other mail. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/bridge: dw-hdmi: fix EDID parsing Date: Thu, 09 Nov 2017 11:49:56 +0200 Message-ID: <87inejaiij.fsf@intel.com> References: <20171109082317.predibr5vv2d33i2@phenom.ffwll.local> <20171109093122.GA12318@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 778846E387 for ; Thu, 9 Nov 2017 09:48:35 +0000 (UTC) In-Reply-To: <20171109093122.GA12318@n2100.armlinux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Russell King - ARM Linux , Daniel Vetter Cc: David Airlie , dri-devel@lists.freedesktop.org, Laurent Pinchart , =?utf-8?Q?Lu=C3=ADs?= Mendes , linux-arm-kernel@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAwOSBOb3YgMjAxNywgUnVzc2VsbCBLaW5nIC0gQVJNIExpbnV4IDxsaW51eEBhcm1s aW51eC5vcmcudWs+IHdyb3RlOgo+IE9uIFRodSwgTm92IDA5LCAyMDE3IGF0IDA5OjIzOjE4QU0g KzAxMDAsIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+IE9uIFR1ZSwgTm92IDA3LCAyMDE3IGF0IDEx OjI3OjIxQU0gKzAwMDAsIFJ1c3NlbGwgS2luZyB3cm90ZToKPj4gPiBQYXJzaW5nIHRoZSBFRElE IGZvciBIRE1JIGFuZCBhdWRpbyBpbmZvcm1hdGlvbiBpbiB0aGUgZ2V0X21vZGVzKCkKPj4gPiBj YWxsYmFjayBpcyBpbmNvcnJlY3QgLSB0aGlzIG9ubHkgcGFyc2VzIHRoZSBFRElEIHJlYWQgZnJv bSB0aGUKPj4gPiBjb25uZWN0b3IsIG5vdCBhbnkgb3ZlcnJpZGUgb3IgZmlybXdhcmUgcHJvdmlk ZWQgRURJRC4KPj4gPiAKPj4gPiBUaGUgY29ycmVjdCBwbGFjZSB0byBwYXJzZSB0aGUgRURJRCBm b3IgdGhlc2UgcGFyYW1ldGVycyBpcyB0aGUKPj4gPiBmaWxsX21vZGVzKCkgY2FsbGJhY2ssIGFm dGVyIHdlJ3ZlIGNhbGxlZCB0aGUgaGVscGVyLiAgTW92ZSB0aGUgcGFyc2luZwo+PiA+IHRoZXJl LiAgVGhpcyBjYXVzZWQgcHJvYmxlbXMgZm9yIEx1w61zIE1lbmRlcy4KPj4gPiAKPj4gPiBDYzog PHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+Cj4+ID4gUmVwb3J0ZWQtYnk6IEx1w61zIE1lbmRlcyA8 bHVpcy5wLm1lbmRlc0BnbWFpbC5jb20+Cj4+ID4gVGVzdGVkLWJ5OiBMdcOtcyBNZW5kZXMgPGx1 aXMucC5tZW5kZXNAZ21haWwuY29tPgo+PiA+IFNpZ25lZC1vZmYtYnk6IFJ1c3NlbGwgS2luZyA8 cm1rK2tlcm5lbEBhcm1saW51eC5vcmcudWs+Cj4+ID4gLS0tCj4+ID4gIGRyaXZlcnMvZ3B1L2Ry bS9icmlkZ2Uvc3lub3BzeXMvZHctaGRtaS5jIHwgMjggKysrKysrKysrKysrKysrKysrKysrKysr LS0tLQo+PiA+ICAxIGZpbGUgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMo LSkKPj4gPiAKPj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5 cy9kdy1oZG1pLmMgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWkuYwo+ PiA+IGluZGV4IDlmZTQwN2Y0OTk4Ni4uMjUxNmExYzE4YTEwIDEwMDY0NAo+PiA+IC0tLSBhL2Ry aXZlcnMvZ3B1L2RybS9icmlkZ2Uvc3lub3BzeXMvZHctaGRtaS5jCj4+ID4gKysrIGIvZHJpdmVy cy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPj4gPiBAQCAtMTkwNSwxMCArMTkw NSw3IEBAIHN0YXRpYyBpbnQgZHdfaGRtaV9jb25uZWN0b3JfZ2V0X21vZGVzKHN0cnVjdCBkcm1f Y29ubmVjdG9yICpjb25uZWN0b3IpCj4+ID4gIAkJZGV2X2RiZyhoZG1pLT5kZXYsICJnb3QgZWRp ZDogd2lkdGhbJWRdIHggaGVpZ2h0WyVkXVxuIiwKPj4gPiAgCQkJZWRpZC0+d2lkdGhfY20sIGVk aWQtPmhlaWdodF9jbSk7Cj4+ID4gIAo+PiA+IC0JCWhkbWktPnNpbmtfaXNfaGRtaSA9IGRybV9k ZXRlY3RfaGRtaV9tb25pdG9yKGVkaWQpOwo+PiA+IC0JCWhkbWktPnNpbmtfaGFzX2F1ZGlvID0g ZHJtX2RldGVjdF9tb25pdG9yX2F1ZGlvKGVkaWQpOwo+PiA+ICAJCWRybV9tb2RlX2Nvbm5lY3Rv cl91cGRhdGVfZWRpZF9wcm9wZXJ0eShjb25uZWN0b3IsIGVkaWQpOwo+PiA+IC0JCWNlY19ub3Rp Zmllcl9zZXRfcGh5c19hZGRyX2Zyb21fZWRpZChoZG1pLT5jZWNfbm90aWZpZXIsIGVkaWQpOwo+ PiA+ICAJCXJldCA9IGRybV9hZGRfZWRpZF9tb2Rlcyhjb25uZWN0b3IsIGVkaWQpOwo+PiA+ICAJ CS8qIFN0b3JlIHRoZSBFTEQgKi8KPj4gPiAgCQlkcm1fZWRpZF90b19lbGQoY29ubmVjdG9yLCBl ZGlkKTsKPj4gPiBAQCAtMTkyMCw2ICsxOTE3LDI5IEBAIHN0YXRpYyBpbnQgZHdfaGRtaV9jb25u ZWN0b3JfZ2V0X21vZGVzKHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3IpCj4+ID4gIAly ZXR1cm4gcmV0Owo+PiA+ICB9Cj4+ID4gIAo+PiA+ICtzdGF0aWMgaW50IGR3X2hkbWlfY29ubmVj dG9yX2ZpbGxfbW9kZXMoc3RydWN0IGRybV9jb25uZWN0b3IgKmNvbm5lY3RvciwKPj4gPiArCQkJ CQl1aW50MzJfdCBtYXhYLCB1aW50MzJfdCBtYXhZKQo+PiA+ICt7Cj4+ID4gKwlzdHJ1Y3QgZHdf aGRtaSAqaGRtaSA9IGNvbnRhaW5lcl9vZihjb25uZWN0b3IsIHN0cnVjdCBkd19oZG1pLAo+PiA+ ICsJCQkJCSAgICBjb25uZWN0b3IpOwo+PiA+ICsJaW50IHJldDsKPj4gPiArCj4+ID4gKwlyZXQg PSBkcm1faGVscGVyX3Byb2JlX3NpbmdsZV9jb25uZWN0b3JfbW9kZXMoY29ubmVjdG9yLCBtYXhY LCBtYXhZKTsKPj4gPiArCj4+ID4gKwlpZiAoY29ubmVjdG9yLT5lZGlkX2Jsb2JfcHRyKSB7Cj4+ ID4gKwkJc3RydWN0IGVkaWQgKmVkaWQgPSAodm9pZCAqKWNvbm5lY3Rvci0+ZWRpZF9ibG9iX3B0 ci0+ZGF0YTsKPj4gPiArCj4+ID4gKwkJaGRtaS0+c2lua19pc19oZG1pID0gZHJtX2RldGVjdF9o ZG1pX21vbml0b3IoZWRpZCk7Cj4+ID4gKwkJaGRtaS0+c2lua19oYXNfYXVkaW8gPSBkcm1fZGV0 ZWN0X21vbml0b3JfYXVkaW8oZWRpZCk7Cj4+ID4gKwkJY2VjX25vdGlmaWVyX3NldF9waHlzX2Fk ZHJfZnJvbV9lZGlkKGhkbWktPmNlY19ub3RpZmllciwgZWRpZCk7Cj4+ID4gKwl9IGVsc2Ugewo+ PiA+ICsJCWhkbWktPnNpbmtfaXNfaGRtaSA9IGZhbHNlOwo+PiA+ICsJCWhkbWktPnNpbmtfaGFz X2F1ZGlvID0gZmFsc2U7Cj4+ID4gKwl9Cj4+ID4gKwo+PiA+ICsJcmV0dXJuIHJldDsKPj4gPiAr fQo+PiA+ICsKPj4gPiAgc3RhdGljIHZvaWQgZHdfaGRtaV9jb25uZWN0b3JfZm9yY2Uoc3RydWN0 IGRybV9jb25uZWN0b3IgKmNvbm5lY3RvcikKPj4gPiAgewo+PiA+ICAJc3RydWN0IGR3X2hkbWkg KmhkbWkgPSBjb250YWluZXJfb2YoY29ubmVjdG9yLCBzdHJ1Y3QgZHdfaGRtaSwKPj4gPiBAQCAt MTkzMyw3ICsxOTUzLDcgQEAgc3RhdGljIHZvaWQgZHdfaGRtaV9jb25uZWN0b3JfZm9yY2Uoc3Ry dWN0IGRybV9jb25uZWN0b3IgKmNvbm5lY3RvcikKPj4gPiAgfQo+PiA+ICAKPj4gPiAgc3RhdGlj IGNvbnN0IHN0cnVjdCBkcm1fY29ubmVjdG9yX2Z1bmNzIGR3X2hkbWlfY29ubmVjdG9yX2Z1bmNz ID0gewo+PiA+IC0JLmZpbGxfbW9kZXMgPSBkcm1faGVscGVyX3Byb2JlX3NpbmdsZV9jb25uZWN0 b3JfbW9kZXMsCj4+ID4gKwkuZmlsbF9tb2RlcyA9IGR3X2hkbWlfY29ubmVjdG9yX2ZpbGxfbW9k ZXMsCj4+IAo+PiBQYXBlcmluZyBvdmVyIGhlbHBlciBmdW5jdGlvbnMgc2hvdWxkbid0IGJlIG5l Y2Vzc2FyeSwgZXhjZXB0IHRoZSBoZWxwZXIKPj4gZnVuY3Rpb25zIG5vdCBoYW5kbGluZyB0aGUg b3ZlcnJpZGUgZWRpZCBpcyBhIGtub3duIGlzc3VlLiBKYW5pIE5pa3VsYSBpcwo+PiB3b3JraW5n IG9uIGEgcHJvcGVyIGZpeCwgcGxlYXNlIGNvb3JkaW5hdGUgd2l0aCBoaW0uCj4KPiBTbywgd2hh dCB5b3UncmUgYmFzaWNhbGx5IHNheWluZyBpcyB0aGF0IGZpeGluZyByZWFsIGJ1Z3MgdGhhdCBh ZmZlY3QKPiB1c2VycyBpcyBub3Qgc29tZXRoaW5nIHRoYXQgRFJNIHBlb3BsZSB3YW50LiAgVGhh dCdzIGZpbmUsIEknbGwgaWdub3JlCj4gcGVvcGxlIHdobyBjb21lIHRvIG1lIGZvciBoZWxwIHdp dGggRFJNIGJ1Z3MgaW4gZnV0dXJlIHRoZW4gYmVjYXVzZQo+IGl0J3Mgb2J2aW91c2x5IGEgZGVh ZCBsb3NzLgoKV2UgbWF5IGFscmVhZHkgaGF2ZSBmaXhlZCB0aGUgYnVnIGluIGRybS1uZXh0IGF0 IHRoZSBwcm9wZXIKbGF5ZXIuIFBsZWFzZSBzZWUgbXkgb3RoZXIgbWFpbC4KCkJSLApKYW5pLgoK LS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcgpfX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFp bGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK