From mboxrd@z Thu Jan 1 00:00:00 1970 From: seanpaul@chromium.org (Sean Paul) Date: Thu, 23 Feb 2017 10:54:37 -0500 Subject: [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check In-Reply-To: <7603150.D9x404uVey@avalon> References: <20161124112231.4297-1-wens@csie.org> <20161206172911.z6sbjzgqv3vfcrfh@lukather> <7603150.D9x404uVey@avalon> Message-ID: <20170223155437.GA24066@art_vandelay> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Dec 07, 2016 at 11:48:55AM +0200, Laurent Pinchart wrote: > Hello, > > On Wednesday 07 Dec 2016 10:26:25 Chen-Yu Tsai wrote: > > On Wed, Dec 7, 2016 at 1:29 AM, Maxime Ripard wrote: > > > On Thu, Nov 24, 2016 at 07:22:31PM +0800, Chen-Yu Tsai wrote: > > >> The panels shipped with Allwinner devices are very "generic", i.e. > > >> they do not have model numbers or reliable sources of information > > >> for the timings (that we know of) other than the fex files shipped > > >> on them. The dot clock frequency provided in the fex files have all > > >> been rounded to the nearest MHz, as that is the unit used in them. > > >> > > >> We were using the simple panel "urt,umsh-8596md-t" as a substitute > > >> for the A13 Q8 tablets in the absence of a specific model for what > > >> may be many different but otherwise timing compatible panels. This > > >> was usable without any visual artifacts or side effects, until the > > >> dot clock rate check was added in commit bb43d40d7c83 ("drm/sun4i: > > >> rgb: Validate the clock rate"). > > >> > > >> The reason this check fails is because the dotclock frequency for > > >> this model is 33.26 MHz, which is not achievable with our dot clock > > >> hardware, and the rate returned by clk_round_rate deviates slightly, > > >> causing the driver to reject the display mode. > > >> > > >> The LCD panels have some tolerance on the dot clock frequency, even > > >> if it's not specified in their datasheets. > > >> > > >> This patch adds a 5% tolerence to the dot clock check. > > > > > > As we discussed already, I really believe this is just as arbitrary as > > > the current behaviour. > > > > Yes. I agree. This patch is mainly to give something that works for > > people who don't care about the details, and to get some feedback > > from people that do. > > > > > Some panels require an exact frequency, > > There's no such thing as an exact frequency, there will always be some > tolerance (and if your display controller can really generate an exact > frequency I'd be very interested in that hardware :-)). There is such thing as exact frequency when you have to worry about panel warranty. We are fortunate, since we can talk to the panel manufacturer and discuss which frequencies are tolerable inside the warranty. We usually hand pick a rate/mode within these constraints. Also, even though things *look* ok on the panel at a certain clock rate, running outside the specified clock could damage the hardware. I don't think it's unreasonable to add tolerances to drm_panel, since they will differ in what is acceptable. The tricky part is teasing out what the tolerances are. Sean > > This is something that has been bugging me for some time now. The problem has > been mostly ignored, or worked around in different ways by different drivers. > I'm afraid I have no generic solution available, but I think we should try to > agree on a common behaviour. > > I don't believe it would be reasonable to request each panel to report a > tolerance, as the value is most of the time not available from the > documentation (when documentation is available). Worse, I'm pretty sure that > most panels documented as fixed timing can actually accept a wide range of > timings. The timings reported in the datasheet are just the nominal values. > > Panels that don't support multiple resolutions obviously require fixed active > h/v values. Even if they can tolerate some departure from the nominal timings > for the sync and porches lengths, it might not be very useful to support that > as I don't expect the display controllers and encoders to be a limiting factor > by not supporting the particular timings that a panel considers as nominal. On > the other hand, departing from the nominal pixel clock frequency is needed as > we can't achieve an exact match, and even possibly to have some control over > the frame rate (although that might also require changing the sync and porches > timings). Without specific information about panel tolerance, do we have any > option other than picking an arbitrary value ? > > > > some have a minimal frequency > > > but no maximum, some have a maximum frequency but no minimal, and I > > > guess most of them deviates by how much exactly they can take (and > > > possibly can take more easily a higher frequency, but are less > > > tolerant if you take a frequency lower than the nominal. > > > > > > And we cannot remove that check entirely, since some bridges will > > > report out of range frequencies for higher modes that we know we > > > cannot reach. > > > > I believe this should be handled by the bridge driver in the check > > callback? The callback I'm changing is attached to the connector, > > which I think doesn't get used if you have a bridge instead. > > And this only checks the pre-registered display modes, such as > > those specified in simple-panel or EDID. > > > > > We could just try to see if the screen pixel clock frequency is out of > > > the pixel clock range we can generate, but then we will loop back on > > > how much out of range is it exactly, and is it within the screen > > > tolerancy. > > > > > > We have an API to deal with the panel tolerancies in the DRM panel > > > framework, we can (and should) use it. > > > > If you mean the get_timings callback, it's not very useful. Most of > > the panels in simple-panel do not use the display_timings structure, > > so they don't return anything. And I get that. The few datasheets > > I found don't list min/max tolerances for the dotclock. > > > > The ones that do have the min/max the same as the recommended value. > > This may or may not be accurate. IIRC the one panel that had this > > that I did check didn't list min/max values in its datasheet. > > > > > I'm not sure how others usually deal with this though. I think I > > > remember Eric telling me that for the RPi they just adjusted the > > > timings a bit, but they only really had a single panel to deal with. > > > > > > Daniel, Eric, Laurent, Sean? Any ideas? > > > > Yes! Feedback please! Between Maxime and me I think we only have a > > limited number of panels, with some overlap. > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check Date: Thu, 23 Feb 2017 10:54:37 -0500 Message-ID: <20170223155437.GA24066@art_vandelay> References: <20161124112231.4297-1-wens@csie.org> <20161206172911.z6sbjzgqv3vfcrfh@lukather> <7603150.D9x404uVey@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-qt0-x22e.google.com (mail-qt0-x22e.google.com [IPv6:2607:f8b0:400d:c0d::22e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7CB916E0D9 for ; Thu, 23 Feb 2017 15:54:41 +0000 (UTC) Received: by mail-qt0-x22e.google.com with SMTP id b16so31937177qte.0 for ; Thu, 23 Feb 2017 07:54:41 -0800 (PST) Content-Disposition: inline In-Reply-To: <7603150.D9x404uVey@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: Daniel Vetter , linux-sunxi , dri-devel , linux-kernel , Chen-Yu Tsai , Maxime Ripard , linux-arm-kernel List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBEZWMgMDcsIDIwMTYgYXQgMTE6NDg6NTVBTSArMDIwMCwgTGF1cmVudCBQaW5jaGFy dCB3cm90ZToKPiBIZWxsbywKPiAKPiBPbiBXZWRuZXNkYXkgMDcgRGVjIDIwMTYgMTA6MjY6MjUg Q2hlbi1ZdSBUc2FpIHdyb3RlOgo+ID4gT24gV2VkLCBEZWMgNywgMjAxNiBhdCAxOjI5IEFNLCBN YXhpbWUgUmlwYXJkIHdyb3RlOgo+ID4gPiBPbiBUaHUsIE5vdiAyNCwgMjAxNiBhdCAwNzoyMjoz MVBNICswODAwLCBDaGVuLVl1IFRzYWkgd3JvdGU6Cj4gPiA+PiBUaGUgcGFuZWxzIHNoaXBwZWQg d2l0aCBBbGx3aW5uZXIgZGV2aWNlcyBhcmUgdmVyeSAiZ2VuZXJpYyIsIGkuZS4KPiA+ID4+IHRo ZXkgZG8gbm90IGhhdmUgbW9kZWwgbnVtYmVycyBvciByZWxpYWJsZSBzb3VyY2VzIG9mIGluZm9y bWF0aW9uCj4gPiA+PiBmb3IgdGhlIHRpbWluZ3MgKHRoYXQgd2Uga25vdyBvZikgb3RoZXIgdGhh biB0aGUgZmV4IGZpbGVzIHNoaXBwZWQKPiA+ID4+IG9uIHRoZW0uIFRoZSBkb3QgY2xvY2sgZnJl cXVlbmN5IHByb3ZpZGVkIGluIHRoZSBmZXggZmlsZXMgaGF2ZSBhbGwKPiA+ID4+IGJlZW4gcm91 bmRlZCB0byB0aGUgbmVhcmVzdCBNSHosIGFzIHRoYXQgaXMgdGhlIHVuaXQgdXNlZCBpbiB0aGVt Lgo+ID4gPj4gCj4gPiA+PiBXZSB3ZXJlIHVzaW5nIHRoZSBzaW1wbGUgcGFuZWwgInVydCx1bXNo LTg1OTZtZC10IiBhcyBhIHN1YnN0aXR1dGUKPiA+ID4+IGZvciB0aGUgQTEzIFE4IHRhYmxldHMg aW4gdGhlIGFic2VuY2Ugb2YgYSBzcGVjaWZpYyBtb2RlbCBmb3Igd2hhdAo+ID4gPj4gbWF5IGJl IG1hbnkgZGlmZmVyZW50IGJ1dCBvdGhlcndpc2UgdGltaW5nIGNvbXBhdGlibGUgcGFuZWxzLiBU aGlzCj4gPiA+PiB3YXMgdXNhYmxlIHdpdGhvdXQgYW55IHZpc3VhbCBhcnRpZmFjdHMgb3Igc2lk ZSBlZmZlY3RzLCB1bnRpbCB0aGUKPiA+ID4+IGRvdCBjbG9jayByYXRlIGNoZWNrIHdhcyBhZGRl ZCBpbiBjb21taXQgYmI0M2Q0MGQ3YzgzICgiZHJtL3N1bjRpOgo+ID4gPj4gcmdiOiBWYWxpZGF0 ZSB0aGUgY2xvY2sgcmF0ZSIpLgo+ID4gPj4gCj4gPiA+PiBUaGUgcmVhc29uIHRoaXMgY2hlY2sg ZmFpbHMgaXMgYmVjYXVzZSB0aGUgZG90Y2xvY2sgZnJlcXVlbmN5IGZvcgo+ID4gPj4gdGhpcyBt b2RlbCBpcyAzMy4yNiBNSHosIHdoaWNoIGlzIG5vdCBhY2hpZXZhYmxlIHdpdGggb3VyIGRvdCBj bG9jawo+ID4gPj4gaGFyZHdhcmUsIGFuZCB0aGUgcmF0ZSByZXR1cm5lZCBieSBjbGtfcm91bmRf cmF0ZSBkZXZpYXRlcyBzbGlnaHRseSwKPiA+ID4+IGNhdXNpbmcgdGhlIGRyaXZlciB0byByZWpl Y3QgdGhlIGRpc3BsYXkgbW9kZS4KPiA+ID4+IAo+ID4gPj4gVGhlIExDRCBwYW5lbHMgaGF2ZSBz b21lIHRvbGVyYW5jZSBvbiB0aGUgZG90IGNsb2NrIGZyZXF1ZW5jeSwgZXZlbgo+ID4gPj4gaWYg aXQncyBub3Qgc3BlY2lmaWVkIGluIHRoZWlyIGRhdGFzaGVldHMuCj4gPiA+PiAKPiA+ID4+IFRo aXMgcGF0Y2ggYWRkcyBhIDUlIHRvbGVyZW5jZSB0byB0aGUgZG90IGNsb2NrIGNoZWNrLgo+ID4g PiAKPiA+ID4gQXMgd2UgZGlzY3Vzc2VkIGFscmVhZHksIEkgcmVhbGx5IGJlbGlldmUgdGhpcyBp cyBqdXN0IGFzIGFyYml0cmFyeSBhcwo+ID4gPiB0aGUgY3VycmVudCBiZWhhdmlvdXIuCj4gPiAK PiA+IFllcy4gSSBhZ3JlZS4gVGhpcyBwYXRjaCBpcyBtYWlubHkgdG8gZ2l2ZSBzb21ldGhpbmcg dGhhdCB3b3JrcyBmb3IKPiA+IHBlb3BsZSB3aG8gZG9uJ3QgY2FyZSBhYm91dCB0aGUgZGV0YWls cywgYW5kIHRvIGdldCBzb21lIGZlZWRiYWNrCj4gPiBmcm9tIHBlb3BsZSB0aGF0IGRvLgo+ID4g Cj4gPiA+IFNvbWUgcGFuZWxzIHJlcXVpcmUgYW4gZXhhY3QgZnJlcXVlbmN5LAo+IAo+IFRoZXJl J3Mgbm8gc3VjaCB0aGluZyBhcyBhbiBleGFjdCBmcmVxdWVuY3ksIHRoZXJlIHdpbGwgYWx3YXlz IGJlIHNvbWUgCj4gdG9sZXJhbmNlIChhbmQgaWYgeW91ciBkaXNwbGF5IGNvbnRyb2xsZXIgY2Fu IHJlYWxseSBnZW5lcmF0ZSBhbiBleGFjdCAKPiBmcmVxdWVuY3kgSSdkIGJlIHZlcnkgaW50ZXJl c3RlZCBpbiB0aGF0IGhhcmR3YXJlIDotKSkuCgpUaGVyZSBpcyBzdWNoIHRoaW5nIGFzIGV4YWN0 IGZyZXF1ZW5jeSB3aGVuIHlvdSBoYXZlIHRvIHdvcnJ5IGFib3V0IHBhbmVsCndhcnJhbnR5LiBX ZSBhcmUgZm9ydHVuYXRlLCBzaW5jZSB3ZSBjYW4gdGFsayB0byB0aGUgcGFuZWwgbWFudWZhY3R1 cmVyIGFuZApkaXNjdXNzIHdoaWNoIGZyZXF1ZW5jaWVzIGFyZSB0b2xlcmFibGUgaW5zaWRlIHRo ZSB3YXJyYW50eS4gV2UgdXN1YWxseSBoYW5kCnBpY2sgYSByYXRlL21vZGUgd2l0aGluIHRoZXNl IGNvbnN0cmFpbnRzLgoKQWxzbywgZXZlbiB0aG91Z2ggdGhpbmdzICpsb29rKiBvayBvbiB0aGUg cGFuZWwgYXQgYSBjZXJ0YWluIGNsb2NrIHJhdGUsIHJ1bm5pbmcKb3V0c2lkZSB0aGUgc3BlY2lm aWVkIGNsb2NrIGNvdWxkIGRhbWFnZSB0aGUgaGFyZHdhcmUuCgpJIGRvbid0IHRoaW5rIGl0J3Mg dW5yZWFzb25hYmxlIHRvIGFkZCB0b2xlcmFuY2VzIHRvIGRybV9wYW5lbCwgc2luY2UgdGhleSB3 aWxsCmRpZmZlciBpbiB3aGF0IGlzIGFjY2VwdGFibGUuIFRoZSB0cmlja3kgcGFydCBpcyB0ZWFz aW5nIG91dCB3aGF0IHRoZSB0b2xlcmFuY2VzCmFyZS4KClNlYW4KCj4gCj4gVGhpcyBpcyBzb21l dGhpbmcgdGhhdCBoYXMgYmVlbiBidWdnaW5nIG1lIGZvciBzb21lIHRpbWUgbm93LiBUaGUgcHJv YmxlbSBoYXMgCj4gYmVlbiBtb3N0bHkgaWdub3JlZCwgb3Igd29ya2VkIGFyb3VuZCBpbiBkaWZm ZXJlbnQgd2F5cyBieSBkaWZmZXJlbnQgZHJpdmVycy4gCj4gSSdtIGFmcmFpZCBJIGhhdmUgbm8g Z2VuZXJpYyBzb2x1dGlvbiBhdmFpbGFibGUsIGJ1dCBJIHRoaW5rIHdlIHNob3VsZCB0cnkgdG8g Cj4gYWdyZWUgb24gYSBjb21tb24gYmVoYXZpb3VyLgo+IAo+IEkgZG9uJ3QgYmVsaWV2ZSBpdCB3 b3VsZCBiZSByZWFzb25hYmxlIHRvIHJlcXVlc3QgZWFjaCBwYW5lbCB0byByZXBvcnQgYSAKPiB0 b2xlcmFuY2UsIGFzIHRoZSB2YWx1ZSBpcyBtb3N0IG9mIHRoZSB0aW1lIG5vdCBhdmFpbGFibGUg ZnJvbSB0aGUgCj4gZG9jdW1lbnRhdGlvbiAod2hlbiBkb2N1bWVudGF0aW9uIGlzIGF2YWlsYWJs ZSkuIFdvcnNlLCBJJ20gcHJldHR5IHN1cmUgdGhhdCAKPiBtb3N0IHBhbmVscyBkb2N1bWVudGVk IGFzIGZpeGVkIHRpbWluZyBjYW4gYWN0dWFsbHkgYWNjZXB0IGEgd2lkZSByYW5nZSBvZiAKPiB0 aW1pbmdzLiBUaGUgdGltaW5ncyByZXBvcnRlZCBpbiB0aGUgZGF0YXNoZWV0IGFyZSBqdXN0IHRo ZSBub21pbmFsIHZhbHVlcy4KPiAKPiBQYW5lbHMgdGhhdCBkb24ndCBzdXBwb3J0IG11bHRpcGxl IHJlc29sdXRpb25zIG9idmlvdXNseSByZXF1aXJlIGZpeGVkIGFjdGl2ZSAKPiBoL3YgdmFsdWVz LiBFdmVuIGlmIHRoZXkgY2FuIHRvbGVyYXRlIHNvbWUgZGVwYXJ0dXJlIGZyb20gdGhlIG5vbWlu YWwgdGltaW5ncyAKPiBmb3IgdGhlIHN5bmMgYW5kIHBvcmNoZXMgbGVuZ3RocywgaXQgbWlnaHQg bm90IGJlIHZlcnkgdXNlZnVsIHRvIHN1cHBvcnQgdGhhdCAKPiBhcyBJIGRvbid0IGV4cGVjdCB0 aGUgZGlzcGxheSBjb250cm9sbGVycyBhbmQgZW5jb2RlcnMgdG8gYmUgYSBsaW1pdGluZyBmYWN0 b3IgCj4gYnkgbm90IHN1cHBvcnRpbmcgdGhlIHBhcnRpY3VsYXIgdGltaW5ncyB0aGF0IGEgcGFu ZWwgY29uc2lkZXJzIGFzIG5vbWluYWwuIE9uIAo+IHRoZSBvdGhlciBoYW5kLCBkZXBhcnRpbmcg ZnJvbSB0aGUgbm9taW5hbCBwaXhlbCBjbG9jayBmcmVxdWVuY3kgaXMgbmVlZGVkIGFzIAo+IHdl IGNhbid0IGFjaGlldmUgYW4gZXhhY3QgbWF0Y2gsIGFuZCBldmVuIHBvc3NpYmx5IHRvIGhhdmUg c29tZSBjb250cm9sIG92ZXIgCj4gdGhlIGZyYW1lIHJhdGUgKGFsdGhvdWdoIHRoYXQgbWlnaHQg YWxzbyByZXF1aXJlIGNoYW5naW5nIHRoZSBzeW5jIGFuZCBwb3JjaGVzIAo+IHRpbWluZ3MpLiBX aXRob3V0IHNwZWNpZmljIGluZm9ybWF0aW9uIGFib3V0IHBhbmVsIHRvbGVyYW5jZSwgZG8gd2Ug aGF2ZSBhbnkgCj4gb3B0aW9uIG90aGVyIHRoYW4gcGlja2luZyBhbiBhcmJpdHJhcnkgdmFsdWUg Pwo+IAo+ID4gPiBzb21lIGhhdmUgYSBtaW5pbWFsIGZyZXF1ZW5jeQo+ID4gPiBidXQgbm8gbWF4 aW11bSwgc29tZSBoYXZlIGEgbWF4aW11bSBmcmVxdWVuY3kgYnV0IG5vIG1pbmltYWwsIGFuZCBJ Cj4gPiA+IGd1ZXNzIG1vc3Qgb2YgdGhlbSBkZXZpYXRlcyBieSBob3cgbXVjaCBleGFjdGx5IHRo ZXkgY2FuIHRha2UgKGFuZAo+ID4gPiBwb3NzaWJseSBjYW4gdGFrZSBtb3JlIGVhc2lseSBhIGhp Z2hlciBmcmVxdWVuY3ksIGJ1dCBhcmUgbGVzcwo+ID4gPiB0b2xlcmFudCBpZiB5b3UgdGFrZSBh IGZyZXF1ZW5jeSBsb3dlciB0aGFuIHRoZSBub21pbmFsLgo+ID4gPiAKPiA+ID4gQW5kIHdlIGNh bm5vdCByZW1vdmUgdGhhdCBjaGVjayBlbnRpcmVseSwgc2luY2Ugc29tZSBicmlkZ2VzIHdpbGwK PiA+ID4gcmVwb3J0IG91dCBvZiByYW5nZSBmcmVxdWVuY2llcyBmb3IgaGlnaGVyIG1vZGVzIHRo YXQgd2Uga25vdyB3ZQo+ID4gPiBjYW5ub3QgcmVhY2guCj4gPiAKPiA+IEkgYmVsaWV2ZSB0aGlz IHNob3VsZCBiZSBoYW5kbGVkIGJ5IHRoZSBicmlkZ2UgZHJpdmVyIGluIHRoZSBjaGVjawo+ID4g Y2FsbGJhY2s/IFRoZSBjYWxsYmFjayBJJ20gY2hhbmdpbmcgaXMgYXR0YWNoZWQgdG8gdGhlIGNv bm5lY3RvciwKPiA+IHdoaWNoIEkgdGhpbmsgZG9lc24ndCBnZXQgdXNlZCBpZiB5b3UgaGF2ZSBh IGJyaWRnZSBpbnN0ZWFkLgo+ID4gQW5kIHRoaXMgb25seSBjaGVja3MgdGhlIHByZS1yZWdpc3Rl cmVkIGRpc3BsYXkgbW9kZXMsIHN1Y2ggYXMKPiA+IHRob3NlIHNwZWNpZmllZCBpbiBzaW1wbGUt cGFuZWwgb3IgRURJRC4KPiA+IAo+ID4gPiBXZSBjb3VsZCBqdXN0IHRyeSB0byBzZWUgaWYgdGhl IHNjcmVlbiBwaXhlbCBjbG9jayBmcmVxdWVuY3kgaXMgb3V0IG9mCj4gPiA+IHRoZSBwaXhlbCBj bG9jayByYW5nZSB3ZSBjYW4gZ2VuZXJhdGUsIGJ1dCB0aGVuIHdlIHdpbGwgbG9vcCBiYWNrIG9u Cj4gPiA+IGhvdyBtdWNoIG91dCBvZiByYW5nZSBpcyBpdCBleGFjdGx5LCBhbmQgaXMgaXQgd2l0 aGluIHRoZSBzY3JlZW4KPiA+ID4gdG9sZXJhbmN5Lgo+ID4gPiAKPiA+ID4gV2UgaGF2ZSBhbiBB UEkgdG8gZGVhbCB3aXRoIHRoZSBwYW5lbCB0b2xlcmFuY2llcyBpbiB0aGUgRFJNIHBhbmVsCj4g PiA+IGZyYW1ld29yaywgd2UgY2FuIChhbmQgc2hvdWxkKSB1c2UgaXQuCj4gPiAKPiA+IElmIHlv dSBtZWFuIHRoZSBnZXRfdGltaW5ncyBjYWxsYmFjaywgaXQncyBub3QgdmVyeSB1c2VmdWwuIE1v c3Qgb2YKPiA+IHRoZSBwYW5lbHMgaW4gc2ltcGxlLXBhbmVsIGRvIG5vdCB1c2UgdGhlIGRpc3Bs YXlfdGltaW5ncyBzdHJ1Y3R1cmUsCj4gPiBzbyB0aGV5IGRvbid0IHJldHVybiBhbnl0aGluZy4g QW5kIEkgZ2V0IHRoYXQuIFRoZSBmZXcgZGF0YXNoZWV0cwo+ID4gSSBmb3VuZCBkb24ndCBsaXN0 IG1pbi9tYXggdG9sZXJhbmNlcyBmb3IgdGhlIGRvdGNsb2NrLgo+ID4gCj4gPiBUaGUgb25lcyB0 aGF0IGRvIGhhdmUgdGhlIG1pbi9tYXggdGhlIHNhbWUgYXMgdGhlIHJlY29tbWVuZGVkIHZhbHVl Lgo+ID4gVGhpcyBtYXkgb3IgbWF5IG5vdCBiZSBhY2N1cmF0ZS4gSUlSQyB0aGUgb25lIHBhbmVs IHRoYXQgaGFkIHRoaXMKPiA+IHRoYXQgSSBkaWQgY2hlY2sgZGlkbid0IGxpc3QgbWluL21heCB2 YWx1ZXMgaW4gaXRzIGRhdGFzaGVldC4KPiA+IAo+ID4gPiBJJ20gbm90IHN1cmUgaG93IG90aGVy cyB1c3VhbGx5IGRlYWwgd2l0aCB0aGlzIHRob3VnaC4gSSB0aGluayBJCj4gPiA+IHJlbWVtYmVy IEVyaWMgdGVsbGluZyBtZSB0aGF0IGZvciB0aGUgUlBpIHRoZXkganVzdCBhZGp1c3RlZCB0aGUK PiA+ID4gdGltaW5ncyBhIGJpdCwgYnV0IHRoZXkgb25seSByZWFsbHkgaGFkIGEgc2luZ2xlIHBh bmVsIHRvIGRlYWwgd2l0aC4KPiA+ID4gCj4gPiA+IERhbmllbCwgRXJpYywgTGF1cmVudCwgU2Vh bj8gQW55IGlkZWFzPwo+ID4gCj4gPiBZZXMhIEZlZWRiYWNrIHBsZWFzZSEgQmV0d2VlbiBNYXhp bWUgYW5kIG1lIEkgdGhpbmsgd2Ugb25seSBoYXZlIGEKPiA+IGxpbWl0ZWQgbnVtYmVyIG9mIHBh bmVscywgd2l0aCBzb21lIG92ZXJsYXAuCj4gCj4gLS0gCj4gUmVnYXJkcywKPiAKPiBMYXVyZW50 IFBpbmNoYXJ0Cj4gCj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX18KPiBkcmktZGV2ZWwgbWFpbGluZyBsaXN0Cj4gZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNr dG9wLm9yZwo+IGh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v ZHJpLWRldmVsCgotLSAKU2VhbiBQYXVsLCBTb2Z0d2FyZSBFbmdpbmVlciwgR29vZ2xlIC8gQ2hy b21pdW0gT1MKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K ZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751644AbdBWPzh (ORCPT ); Thu, 23 Feb 2017 10:55:37 -0500 Received: from mail-qt0-f177.google.com ([209.85.216.177]:33019 "EHLO mail-qt0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751041AbdBWPze (ORCPT ); Thu, 23 Feb 2017 10:55:34 -0500 Date: Thu, 23 Feb 2017 10:54:37 -0500 From: Sean Paul To: Laurent Pinchart Cc: Chen-Yu Tsai , Daniel Vetter , linux-kernel , dri-devel , linux-sunxi , Maxime Ripard , linux-arm-kernel Subject: Re: [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check Message-ID: <20170223155437.GA24066@art_vandelay> References: <20161124112231.4297-1-wens@csie.org> <20161206172911.z6sbjzgqv3vfcrfh@lukather> <7603150.D9x404uVey@avalon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7603150.D9x404uVey@avalon> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 07, 2016 at 11:48:55AM +0200, Laurent Pinchart wrote: > Hello, > > On Wednesday 07 Dec 2016 10:26:25 Chen-Yu Tsai wrote: > > On Wed, Dec 7, 2016 at 1:29 AM, Maxime Ripard wrote: > > > On Thu, Nov 24, 2016 at 07:22:31PM +0800, Chen-Yu Tsai wrote: > > >> The panels shipped with Allwinner devices are very "generic", i.e. > > >> they do not have model numbers or reliable sources of information > > >> for the timings (that we know of) other than the fex files shipped > > >> on them. The dot clock frequency provided in the fex files have all > > >> been rounded to the nearest MHz, as that is the unit used in them. > > >> > > >> We were using the simple panel "urt,umsh-8596md-t" as a substitute > > >> for the A13 Q8 tablets in the absence of a specific model for what > > >> may be many different but otherwise timing compatible panels. This > > >> was usable without any visual artifacts or side effects, until the > > >> dot clock rate check was added in commit bb43d40d7c83 ("drm/sun4i: > > >> rgb: Validate the clock rate"). > > >> > > >> The reason this check fails is because the dotclock frequency for > > >> this model is 33.26 MHz, which is not achievable with our dot clock > > >> hardware, and the rate returned by clk_round_rate deviates slightly, > > >> causing the driver to reject the display mode. > > >> > > >> The LCD panels have some tolerance on the dot clock frequency, even > > >> if it's not specified in their datasheets. > > >> > > >> This patch adds a 5% tolerence to the dot clock check. > > > > > > As we discussed already, I really believe this is just as arbitrary as > > > the current behaviour. > > > > Yes. I agree. This patch is mainly to give something that works for > > people who don't care about the details, and to get some feedback > > from people that do. > > > > > Some panels require an exact frequency, > > There's no such thing as an exact frequency, there will always be some > tolerance (and if your display controller can really generate an exact > frequency I'd be very interested in that hardware :-)). There is such thing as exact frequency when you have to worry about panel warranty. We are fortunate, since we can talk to the panel manufacturer and discuss which frequencies are tolerable inside the warranty. We usually hand pick a rate/mode within these constraints. Also, even though things *look* ok on the panel at a certain clock rate, running outside the specified clock could damage the hardware. I don't think it's unreasonable to add tolerances to drm_panel, since they will differ in what is acceptable. The tricky part is teasing out what the tolerances are. Sean > > This is something that has been bugging me for some time now. The problem has > been mostly ignored, or worked around in different ways by different drivers. > I'm afraid I have no generic solution available, but I think we should try to > agree on a common behaviour. > > I don't believe it would be reasonable to request each panel to report a > tolerance, as the value is most of the time not available from the > documentation (when documentation is available). Worse, I'm pretty sure that > most panels documented as fixed timing can actually accept a wide range of > timings. The timings reported in the datasheet are just the nominal values. > > Panels that don't support multiple resolutions obviously require fixed active > h/v values. Even if they can tolerate some departure from the nominal timings > for the sync and porches lengths, it might not be very useful to support that > as I don't expect the display controllers and encoders to be a limiting factor > by not supporting the particular timings that a panel considers as nominal. On > the other hand, departing from the nominal pixel clock frequency is needed as > we can't achieve an exact match, and even possibly to have some control over > the frame rate (although that might also require changing the sync and porches > timings). Without specific information about panel tolerance, do we have any > option other than picking an arbitrary value ? > > > > some have a minimal frequency > > > but no maximum, some have a maximum frequency but no minimal, and I > > > guess most of them deviates by how much exactly they can take (and > > > possibly can take more easily a higher frequency, but are less > > > tolerant if you take a frequency lower than the nominal. > > > > > > And we cannot remove that check entirely, since some bridges will > > > report out of range frequencies for higher modes that we know we > > > cannot reach. > > > > I believe this should be handled by the bridge driver in the check > > callback? The callback I'm changing is attached to the connector, > > which I think doesn't get used if you have a bridge instead. > > And this only checks the pre-registered display modes, such as > > those specified in simple-panel or EDID. > > > > > We could just try to see if the screen pixel clock frequency is out of > > > the pixel clock range we can generate, but then we will loop back on > > > how much out of range is it exactly, and is it within the screen > > > tolerancy. > > > > > > We have an API to deal with the panel tolerancies in the DRM panel > > > framework, we can (and should) use it. > > > > If you mean the get_timings callback, it's not very useful. Most of > > the panels in simple-panel do not use the display_timings structure, > > so they don't return anything. And I get that. The few datasheets > > I found don't list min/max tolerances for the dotclock. > > > > The ones that do have the min/max the same as the recommended value. > > This may or may not be accurate. IIRC the one panel that had this > > that I did check didn't list min/max values in its datasheet. > > > > > I'm not sure how others usually deal with this though. I think I > > > remember Eric telling me that for the RPi they just adjusted the > > > timings a bit, but they only really had a single panel to deal with. > > > > > > Daniel, Eric, Laurent, Sean? Any ideas? > > > > Yes! Feedback please! Between Maxime and me I think we only have a > > limited number of panels, with some overlap. > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS