From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:40142 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725797AbeLDSvK (ORCPT ); Tue, 4 Dec 2018 13:51:10 -0500 From: Laurent Pinchart To: Geert Uytterhoeven Cc: Laurent Pinchart , DRI Development , Linux-Renesas , Kieran Bingham Subject: Re: [PATCH v2] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency Date: Tue, 04 Dec 2018 20:51:41 +0200 Message-ID: <4080472.x6oRvDE9so@avalon> In-Reply-To: References: <20181204163640.316-1-laurent.pinchart+renesas@ideasonboard.com> <7465409.qeCXr2vvpE@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Geert, On Tuesday, 4 December 2018 20:42:53 EET Geert Uytterhoeven wrote: > On Tue, Dec 4, 2018 at 7:12 PM Laurent Pinchart wrote: > > On Tuesday, 4 December 2018 19:30:25 EET Geert Uytterhoeven wrote: > >> On Tue, Dec 4, 2018 at 5:36 PM Laurent Pinchart wrote: > >>> Implement a .mode_valid() handler in the R-Car glue layer to reject > >>> modes with an unsupported clock frequency. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >> > >> Thanks for your patch! > >> > >>> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > >>> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c > >>> @@ -35,6 +35,20 @@ static const struct rcar_hdmi_phy_params > >>> rcar_hdmi_phy_params[] = { > >>> { ~0UL, 0x0000, 0x0000, 0x0000 }, > >>> }; > >>> > >>> +static enum drm_mode_status > >>> +rcar_hdmi_mode_valid(struct drm_connector *connector, > >>> + const struct drm_display_mode *mode) > >>> +{ > >>> + /* > >>> + * The maximum supported clock frequency is 297 MHz, as shown > >>> in the PHY > >>> + * parameters table. > >>> + */ > >>> + if (mode->clock > 297000) > >>> + return MODE_CLOCK_HIGH; > >> > >> Perhaps you need a check for the lower limit (25 MHz), too? > > > > There's no lower limit implied by the rcar_hdmi_phy_params table. > > Oh, you mean the table in the driver, not a table in the Hardware User's > Manual? Correct, I mean the table in the driver. This patch was prompted by an error returned from rcar_hdmi_phy_configure() when the mode frequency was too high, making mode setting failed. I've thus added a .mode_valid() handler to ensure that invalid modes don't get exposed to upper layers, fixing such use cases as fbvon on a 4K monitor (where the fbcon was picking a mode advertised as supported by the driver while its frequency was too high). > That's why I couldn't find the table, but only a short notice in the HDMI > section of the Hardware User's Manual, stating: > > Pixel clock from 25MHz up to 297MHz Well, the IP core vendor doesn't allow us to submit patches based on the content of non-public documentation, so I'm afraid I won't sign such a patch without being given explicit permission. It's a very stupid game really, but I don't set the rules :-( -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency Date: Tue, 04 Dec 2018 20:51:41 +0200 Message-ID: <4080472.x6oRvDE9so@avalon> References: <20181204163640.316-1-laurent.pinchart+renesas@ideasonboard.com> <7465409.qeCXr2vvpE@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id DF8906E2F9 for ; Tue, 4 Dec 2018 18:51:10 +0000 (UTC) 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: Geert Uytterhoeven Cc: Linux-Renesas , Laurent Pinchart , Kieran Bingham , DRI Development List-Id: dri-devel@lists.freedesktop.org SGkgR2VlcnQsCgpPbiBUdWVzZGF5LCA0IERlY2VtYmVyIDIwMTggMjA6NDI6NTMgRUVUIEdlZXJ0 IFV5dHRlcmhvZXZlbiB3cm90ZToKPiBPbiBUdWUsIERlYyA0LCAyMDE4IGF0IDc6MTIgUE0gTGF1 cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9uIFR1ZXNkYXksIDQgRGVjZW1iZXIgMjAxOCAxOToz MDoyNSBFRVQgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3RlOgo+ID4+IE9uIFR1ZSwgRGVjIDQsIDIw MTggYXQgNTozNiBQTSBMYXVyZW50IFBpbmNoYXJ0IHdyb3RlOgo+ID4+PiBJbXBsZW1lbnQgYSAu bW9kZV92YWxpZCgpIGhhbmRsZXIgaW4gdGhlIFItQ2FyIGdsdWUgbGF5ZXIgdG8gcmVqZWN0Cj4g Pj4+IG1vZGVzIHdpdGggYW4gdW5zdXBwb3J0ZWQgY2xvY2sgZnJlcXVlbmN5Lgo+ID4+PiAKPiA+ Pj4gU2lnbmVkLW9mZi1ieTogTGF1cmVudCBQaW5jaGFydAo+ID4+PiA8bGF1cmVudC5waW5jaGFy dCtyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+Cj4gPj4gCj4gPj4gVGhhbmtzIGZvciB5b3VyIHBh dGNoIQo+ID4+IAo+ID4+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R3X2hk bWkuYwo+ID4+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R3X2hkbWkuYwo+ ID4+PiBAQCAtMzUsNiArMzUsMjAgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCByY2FyX2hkbWlfcGh5 X3BhcmFtcwo+ID4+PiByY2FyX2hkbWlfcGh5X3BhcmFtc1tdID0gewo+ID4+PiAgICAgICAgIHsg fjBVTCwgICAgICAweDAwMDAsIDB4MDAwMCwgMHgwMDAwIH0sCj4gPj4+ICB9Owo+ID4+PiAKPiA+ Pj4gK3N0YXRpYyBlbnVtIGRybV9tb2RlX3N0YXR1cwo+ID4+PiArcmNhcl9oZG1pX21vZGVfdmFs aWQoc3RydWN0IGRybV9jb25uZWN0b3IgKmNvbm5lY3RvciwKPiA+Pj4gKyAgICAgICAgICAgICAg ICAgICAgY29uc3Qgc3RydWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUpCj4gPj4+ICt7Cj4gPj4+ ICsgICAgICAgLyoKPiA+Pj4gKyAgICAgICAgKiBUaGUgbWF4aW11bSBzdXBwb3J0ZWQgY2xvY2sg ZnJlcXVlbmN5IGlzIDI5NyBNSHosIGFzIHNob3duCj4gPj4+IGluIHRoZSBQSFkKPiA+Pj4gKyAg ICAgICAgKiBwYXJhbWV0ZXJzIHRhYmxlLgo+ID4+PiArICAgICAgICAqLwo+ID4+PiArICAgICAg IGlmIChtb2RlLT5jbG9jayA+IDI5NzAwMCkKPiA+Pj4gKyAgICAgICAgICAgICAgIHJldHVybiBN T0RFX0NMT0NLX0hJR0g7Cj4gPj4gCj4gPj4gUGVyaGFwcyB5b3UgbmVlZCBhIGNoZWNrIGZvciB0 aGUgbG93ZXIgbGltaXQgKDI1IE1IeiksIHRvbz8KPiA+IAo+ID4gVGhlcmUncyBubyBsb3dlciBs aW1pdCBpbXBsaWVkIGJ5IHRoZSByY2FyX2hkbWlfcGh5X3BhcmFtcyB0YWJsZS4KPiAKPiBPaCwg eW91IG1lYW4gdGhlIHRhYmxlIGluIHRoZSBkcml2ZXIsIG5vdCBhIHRhYmxlIGluIHRoZSBIYXJk d2FyZSBVc2VyJ3MKPiBNYW51YWw/CgpDb3JyZWN0LCBJIG1lYW4gdGhlIHRhYmxlIGluIHRoZSBk cml2ZXIuIFRoaXMgcGF0Y2ggd2FzIHByb21wdGVkIGJ5IGFuIGVycm9yIApyZXR1cm5lZCBmcm9t IHJjYXJfaGRtaV9waHlfY29uZmlndXJlKCkgd2hlbiB0aGUgbW9kZSBmcmVxdWVuY3kgd2FzIHRv byBoaWdoLCAKbWFraW5nIG1vZGUgc2V0dGluZyBmYWlsZWQuIEkndmUgdGh1cyBhZGRlZCBhIC5t b2RlX3ZhbGlkKCkgaGFuZGxlciB0byBlbnN1cmUgCnRoYXQgaW52YWxpZCBtb2RlcyBkb24ndCBn ZXQgZXhwb3NlZCB0byB1cHBlciBsYXllcnMsIGZpeGluZyBzdWNoIHVzZSBjYXNlcyBhcyAKZmJ2 b24gb24gYSA0SyBtb25pdG9yICh3aGVyZSB0aGUgZmJjb24gd2FzIHBpY2tpbmcgYSBtb2RlIGFk dmVydGlzZWQgYXMgCnN1cHBvcnRlZCBieSB0aGUgZHJpdmVyIHdoaWxlIGl0cyBmcmVxdWVuY3kg d2FzIHRvbyBoaWdoKS4KCj4gVGhhdCdzIHdoeSBJIGNvdWxkbid0IGZpbmQgdGhlIHRhYmxlLCBi dXQgb25seSBhIHNob3J0IG5vdGljZSBpbiB0aGUgSERNSQo+IHNlY3Rpb24gb2YgdGhlIEhhcmR3 YXJlIFVzZXIncyBNYW51YWwsIHN0YXRpbmc6Cj4gCj4gICAgIFBpeGVsIGNsb2NrIGZyb20gMjVN SHogdXAgdG8gMjk3TUh6CgpXZWxsLCB0aGUgSVAgY29yZSB2ZW5kb3IgZG9lc24ndCBhbGxvdyB1 cyB0byBzdWJtaXQgcGF0Y2hlcyBiYXNlZCBvbiB0aGUgCmNvbnRlbnQgb2Ygbm9uLXB1YmxpYyBk b2N1bWVudGF0aW9uLCBzbyBJJ20gYWZyYWlkIEkgd29uJ3Qgc2lnbiBzdWNoIGEgcGF0Y2ggCndp dGhvdXQgYmVpbmcgZ2l2ZW4gZXhwbGljaXQgcGVybWlzc2lvbi4gSXQncyBhIHZlcnkgc3R1cGlk IGdhbWUgcmVhbGx5LCBidXQgSSAKZG9uJ3Qgc2V0IHRoZSBydWxlcyA6LSgKCi0tIApSZWdhcmRz LAoKTGF1cmVudCBQaW5jaGFydAoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWwK