From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Mon, 27 Feb 2017 10:26:44 +0200 Subject: [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check In-Reply-To: <20170223155437.GA24066@art_vandelay> References: <20161124112231.4297-1-wens@csie.org> <7603150.D9x404uVey@avalon> <20170223155437.GA24066@art_vandelay> Message-ID: <2092223.yo6q8ldtYz@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Sean, On Thursday 23 Feb 2017 10:54:37 Sean Paul wrote: > On Wed, Dec 07, 2016 at 11:48:55AM +0200, Laurent Pinchart wrote: > > 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. There's no such thing as exact frequency when you live in a world that has to comply with the laws of physics :-) This would require an infinitely precise clock, and there's no such thing. > 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'm curious, how so ? What happens to the panel if you feed it a clock outside of that range ? > 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. More than tricky, in the vast majority of cases we don't have that information at all :-/ > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check Date: Mon, 27 Feb 2017 10:26:44 +0200 Message-ID: <2092223.yo6q8ldtYz@avalon> References: <20161124112231.4297-1-wens@csie.org> <7603150.D9x404uVey@avalon> <20170223155437.GA24066@art_vandelay> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 07D3B89C80 for ; Mon, 27 Feb 2017 08:26:13 +0000 (UTC) In-Reply-To: <20170223155437.GA24066@art_vandelay> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sean Paul Cc: Daniel Vetter , linux-sunxi , dri-devel , linux-kernel , Chen-Yu Tsai , Maxime Ripard , linux-arm-kernel List-Id: dri-devel@lists.freedesktop.org SGkgU2VhbiwKCk9uIFRodXJzZGF5IDIzIEZlYiAyMDE3IDEwOjU0OjM3IFNlYW4gUGF1bCB3cm90 ZToKPiBPbiBXZWQsIERlYyAwNywgMjAxNiBhdCAxMTo0ODo1NUFNICswMjAwLCBMYXVyZW50IFBp bmNoYXJ0IHdyb3RlOgo+ID4gT24gV2VkbmVzZGF5IDA3IERlYyAyMDE2IDEwOjI2OjI1IENoZW4t WXUgVHNhaSB3cm90ZToKPiA+PiBPbiBXZWQsIERlYyA3LCAyMDE2IGF0IDE6MjkgQU0sIE1heGlt ZSBSaXBhcmQgd3JvdGU6Cj4gPj4+IE9uIFRodSwgTm92IDI0LCAyMDE2IGF0IDA3OjIyOjMxUE0g KzA4MDAsIENoZW4tWXUgVHNhaSB3cm90ZToKPiA+Pj4+IFRoZSBwYW5lbHMgc2hpcHBlZCB3aXRo IEFsbHdpbm5lciBkZXZpY2VzIGFyZSB2ZXJ5ICJnZW5lcmljIiwgaS5lLgo+ID4+Pj4gdGhleSBk byBub3QgaGF2ZSBtb2RlbCBudW1iZXJzIG9yIHJlbGlhYmxlIHNvdXJjZXMgb2YgaW5mb3JtYXRp b24KPiA+Pj4+IGZvciB0aGUgdGltaW5ncyAodGhhdCB3ZSBrbm93IG9mKSBvdGhlciB0aGFuIHRo ZSBmZXggZmlsZXMgc2hpcHBlZAo+ID4+Pj4gb24gdGhlbS4gVGhlIGRvdCBjbG9jayBmcmVxdWVu Y3kgcHJvdmlkZWQgaW4gdGhlIGZleCBmaWxlcyBoYXZlIGFsbAo+ID4+Pj4gYmVlbiByb3VuZGVk IHRvIHRoZSBuZWFyZXN0IE1IeiwgYXMgdGhhdCBpcyB0aGUgdW5pdCB1c2VkIGluIHRoZW0uCj4g Pj4+PiAKPiA+Pj4+IFdlIHdlcmUgdXNpbmcgdGhlIHNpbXBsZSBwYW5lbCAidXJ0LHVtc2gtODU5 Nm1kLXQiIGFzIGEgc3Vic3RpdHV0ZQo+ID4+Pj4gZm9yIHRoZSBBMTMgUTggdGFibGV0cyBpbiB0 aGUgYWJzZW5jZSBvZiBhIHNwZWNpZmljIG1vZGVsIGZvciB3aGF0Cj4gPj4+PiBtYXkgYmUgbWFu eSBkaWZmZXJlbnQgYnV0IG90aGVyd2lzZSB0aW1pbmcgY29tcGF0aWJsZSBwYW5lbHMuIFRoaXMK PiA+Pj4+IHdhcyB1c2FibGUgd2l0aG91dCBhbnkgdmlzdWFsIGFydGlmYWN0cyBvciBzaWRlIGVm ZmVjdHMsIHVudGlsIHRoZQo+ID4+Pj4gZG90IGNsb2NrIHJhdGUgY2hlY2sgd2FzIGFkZGVkIGlu IGNvbW1pdCBiYjQzZDQwZDdjODMgKCJkcm0vc3VuNGk6Cj4gPj4+PiByZ2I6IFZhbGlkYXRlIHRo ZSBjbG9jayByYXRlIikuCj4gPj4+PiAKPiA+Pj4+IFRoZSByZWFzb24gdGhpcyBjaGVjayBmYWls cyBpcyBiZWNhdXNlIHRoZSBkb3RjbG9jayBmcmVxdWVuY3kgZm9yCj4gPj4+PiB0aGlzIG1vZGVs IGlzIDMzLjI2IE1Ieiwgd2hpY2ggaXMgbm90IGFjaGlldmFibGUgd2l0aCBvdXIgZG90IGNsb2Nr Cj4gPj4+PiBoYXJkd2FyZSwgYW5kIHRoZSByYXRlIHJldHVybmVkIGJ5IGNsa19yb3VuZF9yYXRl IGRldmlhdGVzIHNsaWdodGx5LAo+ID4+Pj4gY2F1c2luZyB0aGUgZHJpdmVyIHRvIHJlamVjdCB0 aGUgZGlzcGxheSBtb2RlLgo+ID4+Pj4gCj4gPj4+PiBUaGUgTENEIHBhbmVscyBoYXZlIHNvbWUg dG9sZXJhbmNlIG9uIHRoZSBkb3QgY2xvY2sgZnJlcXVlbmN5LCBldmVuCj4gPj4+PiBpZiBpdCdz IG5vdCBzcGVjaWZpZWQgaW4gdGhlaXIgZGF0YXNoZWV0cy4KPiA+Pj4+IAo+ID4+Pj4gVGhpcyBw YXRjaCBhZGRzIGEgNSUgdG9sZXJlbmNlIHRvIHRoZSBkb3QgY2xvY2sgY2hlY2suCj4gPj4+IAo+ ID4+PiBBcyB3ZSBkaXNjdXNzZWQgYWxyZWFkeSwgSSByZWFsbHkgYmVsaWV2ZSB0aGlzIGlzIGp1 c3QgYXMgYXJiaXRyYXJ5IGFzCj4gPj4+IHRoZSBjdXJyZW50IGJlaGF2aW91ci4KPiA+PiAKPiA+ PiBZZXMuIEkgYWdyZWUuIFRoaXMgcGF0Y2ggaXMgbWFpbmx5IHRvIGdpdmUgc29tZXRoaW5nIHRo YXQgd29ya3MgZm9yCj4gPj4gcGVvcGxlIHdobyBkb24ndCBjYXJlIGFib3V0IHRoZSBkZXRhaWxz LCBhbmQgdG8gZ2V0IHNvbWUgZmVlZGJhY2sKPiA+PiBmcm9tIHBlb3BsZSB0aGF0IGRvLgo+ID4+ IAo+ID4+PiBTb21lIHBhbmVscyByZXF1aXJlIGFuIGV4YWN0IGZyZXF1ZW5jeSwKPiA+IAo+ID4g VGhlcmUncyBubyBzdWNoIHRoaW5nIGFzIGFuIGV4YWN0IGZyZXF1ZW5jeSwgdGhlcmUgd2lsbCBh bHdheXMgYmUgc29tZQo+ID4gdG9sZXJhbmNlIChhbmQgaWYgeW91ciBkaXNwbGF5IGNvbnRyb2xs ZXIgY2FuIHJlYWxseSBnZW5lcmF0ZSBhbiBleGFjdAo+ID4gZnJlcXVlbmN5IEknZCBiZSB2ZXJ5 IGludGVyZXN0ZWQgaW4gdGhhdCBoYXJkd2FyZSA6LSkpLgo+IAo+IFRoZXJlIGlzIHN1Y2ggdGhp bmcgYXMgZXhhY3QgZnJlcXVlbmN5IHdoZW4geW91IGhhdmUgdG8gd29ycnkgYWJvdXQgcGFuZWwK PiB3YXJyYW50eS4KClRoZXJlJ3Mgbm8gc3VjaCB0aGluZyBhcyBleGFjdCBmcmVxdWVuY3kgd2hl biB5b3UgbGl2ZSBpbiBhIHdvcmxkIHRoYXQgaGFzIHRvIApjb21wbHkgd2l0aCB0aGUgbGF3cyBv ZiBwaHlzaWNzIDotKSBUaGlzIHdvdWxkIHJlcXVpcmUgYW4gaW5maW5pdGVseSBwcmVjaXNlIApj bG9jaywgYW5kIHRoZXJlJ3Mgbm8gc3VjaCB0aGluZy4KCj4gV2UgYXJlIGZvcnR1bmF0ZSwgc2lu Y2Ugd2UgY2FuIHRhbGsgdG8gdGhlIHBhbmVsIG1hbnVmYWN0dXJlciBhbmQgZGlzY3Vzcwo+IHdo aWNoIGZyZXF1ZW5jaWVzIGFyZSB0b2xlcmFibGUgaW5zaWRlIHRoZSB3YXJyYW50eS4gV2UgdXN1 YWxseSBoYW5kIHBpY2sgYQo+IHJhdGUvbW9kZSB3aXRoaW4gdGhlc2UgY29uc3RyYWludHMuCj4K PiBBbHNvLCBldmVuIHRob3VnaCB0aGluZ3MgKmxvb2sqIG9rIG9uIHRoZSBwYW5lbCBhdCBhIGNl cnRhaW4gY2xvY2sgcmF0ZSwKPiBydW5uaW5nIG91dHNpZGUgdGhlIHNwZWNpZmllZCBjbG9jayBj b3VsZCBkYW1hZ2UgdGhlIGhhcmR3YXJlLgoKSSdtIGN1cmlvdXMsIGhvdyBzbyA/IFdoYXQgaGFw cGVucyB0byB0aGUgcGFuZWwgaWYgeW91IGZlZWQgaXQgYSBjbG9jayBvdXRzaWRlIApvZiB0aGF0 IHJhbmdlID8KCj4gSSBkb24ndCB0aGluayBpdCdzIHVucmVhc29uYWJsZSB0byBhZGQgdG9sZXJh bmNlcyB0byBkcm1fcGFuZWwsIHNpbmNlIHRoZXkKPiB3aWxsIGRpZmZlciBpbiB3aGF0IGlzIGFj Y2VwdGFibGUuIFRoZSB0cmlja3kgcGFydCBpcyB0ZWFzaW5nIG91dCB3aGF0IHRoZQo+IHRvbGVy YW5jZXMgYXJlLgoKTW9yZSB0aGFuIHRyaWNreSwgaW4gdGhlIHZhc3QgbWFqb3JpdHkgb2YgY2Fz ZXMgd2UgZG9uJ3QgaGF2ZSB0aGF0IGluZm9ybWF0aW9uIAphdCBhbGwgOi0vCgo+ID4gVGhpcyBp cyBzb21ldGhpbmcgdGhhdCBoYXMgYmVlbiBidWdnaW5nIG1lIGZvciBzb21lIHRpbWUgbm93LiBU aGUgcHJvYmxlbQo+ID4gaGFzIGJlZW4gbW9zdGx5IGlnbm9yZWQsIG9yIHdvcmtlZCBhcm91bmQg aW4gZGlmZmVyZW50IHdheXMgYnkgZGlmZmVyZW50Cj4gPiBkcml2ZXJzLiBJJ20gYWZyYWlkIEkg aGF2ZSBubyBnZW5lcmljIHNvbHV0aW9uIGF2YWlsYWJsZSwgYnV0IEkgdGhpbmsgd2UKPiA+IHNo b3VsZCB0cnkgdG8gYWdyZWUgb24gYSBjb21tb24gYmVoYXZpb3VyLgo+ID4gCj4gPiBJIGRvbid0 IGJlbGlldmUgaXQgd291bGQgYmUgcmVhc29uYWJsZSB0byByZXF1ZXN0IGVhY2ggcGFuZWwgdG8g cmVwb3J0IGEKPiA+IHRvbGVyYW5jZSwgYXMgdGhlIHZhbHVlIGlzIG1vc3Qgb2YgdGhlIHRpbWUg bm90IGF2YWlsYWJsZSBmcm9tIHRoZQo+ID4gZG9jdW1lbnRhdGlvbiAod2hlbiBkb2N1bWVudGF0 aW9uIGlzIGF2YWlsYWJsZSkuIFdvcnNlLCBJJ20gcHJldHR5IHN1cmUKPiA+IHRoYXQgbW9zdCBw YW5lbHMgZG9jdW1lbnRlZCBhcyBmaXhlZCB0aW1pbmcgY2FuIGFjdHVhbGx5IGFjY2VwdCBhIHdp ZGUKPiA+IHJhbmdlIG9mIHRpbWluZ3MuIFRoZSB0aW1pbmdzIHJlcG9ydGVkIGluIHRoZSBkYXRh c2hlZXQgYXJlIGp1c3QgdGhlCj4gPiBub21pbmFsIHZhbHVlcy4KPiA+IAo+ID4gUGFuZWxzIHRo YXQgZG9uJ3Qgc3VwcG9ydCBtdWx0aXBsZSByZXNvbHV0aW9ucyBvYnZpb3VzbHkgcmVxdWlyZSBm aXhlZAo+ID4gYWN0aXZlIGgvdiB2YWx1ZXMuIEV2ZW4gaWYgdGhleSBjYW4gdG9sZXJhdGUgc29t ZSBkZXBhcnR1cmUgZnJvbSB0aGUKPiA+IG5vbWluYWwgdGltaW5ncyBmb3IgdGhlIHN5bmMgYW5k IHBvcmNoZXMgbGVuZ3RocywgaXQgbWlnaHQgbm90IGJlIHZlcnkKPiA+IHVzZWZ1bCB0byBzdXBw b3J0IHRoYXQgYXMgSSBkb24ndCBleHBlY3QgdGhlIGRpc3BsYXkgY29udHJvbGxlcnMgYW5kCj4g PiBlbmNvZGVycyB0byBiZSBhIGxpbWl0aW5nIGZhY3RvciBieSBub3Qgc3VwcG9ydGluZyB0aGUg cGFydGljdWxhciB0aW1pbmdzCj4gPiB0aGF0IGEgcGFuZWwgY29uc2lkZXJzIGFzIG5vbWluYWwu IE9uIHRoZSBvdGhlciBoYW5kLCBkZXBhcnRpbmcgZnJvbSB0aGUKPiA+IG5vbWluYWwgcGl4ZWwg Y2xvY2sgZnJlcXVlbmN5IGlzIG5lZWRlZCBhcyB3ZSBjYW4ndCBhY2hpZXZlIGFuIGV4YWN0Cj4g PiBtYXRjaCwgYW5kIGV2ZW4gcG9zc2libHkgdG8gaGF2ZSBzb21lIGNvbnRyb2wgb3ZlciB0aGUg ZnJhbWUgcmF0ZQo+ID4gKGFsdGhvdWdoIHRoYXQgbWlnaHQgYWxzbyByZXF1aXJlIGNoYW5naW5n IHRoZSBzeW5jIGFuZCBwb3JjaGVzIHRpbWluZ3MpLgo+ID4gV2l0aG91dCBzcGVjaWZpYyBpbmZv cm1hdGlvbiBhYm91dCBwYW5lbCB0b2xlcmFuY2UsIGRvIHdlIGhhdmUgYW55IG9wdGlvbgo+ID4g b3RoZXIgdGhhbiBwaWNraW5nIGFuIGFyYml0cmFyeSB2YWx1ZSA/Cj4gPiAKPiA+Pj4gc29tZSBo YXZlIGEgbWluaW1hbCBmcmVxdWVuY3kKPiA+Pj4gYnV0IG5vIG1heGltdW0sIHNvbWUgaGF2ZSBh IG1heGltdW0gZnJlcXVlbmN5IGJ1dCBubyBtaW5pbWFsLCBhbmQgSQo+ID4+PiBndWVzcyBtb3N0 IG9mIHRoZW0gZGV2aWF0ZXMgYnkgaG93IG11Y2ggZXhhY3RseSB0aGV5IGNhbiB0YWtlIChhbmQK PiA+Pj4gcG9zc2libHkgY2FuIHRha2UgbW9yZSBlYXNpbHkgYSBoaWdoZXIgZnJlcXVlbmN5LCBi dXQgYXJlIGxlc3MKPiA+Pj4gdG9sZXJhbnQgaWYgeW91IHRha2UgYSBmcmVxdWVuY3kgbG93ZXIg dGhhbiB0aGUgbm9taW5hbC4KPiA+Pj4gCj4gPj4+IEFuZCB3ZSBjYW5ub3QgcmVtb3ZlIHRoYXQg Y2hlY2sgZW50aXJlbHksIHNpbmNlIHNvbWUgYnJpZGdlcyB3aWxsCj4gPj4+IHJlcG9ydCBvdXQg b2YgcmFuZ2UgZnJlcXVlbmNpZXMgZm9yIGhpZ2hlciBtb2RlcyB0aGF0IHdlIGtub3cgd2UKPiA+ Pj4gY2Fubm90IHJlYWNoLgo+ID4+IAo+ID4+IEkgYmVsaWV2ZSB0aGlzIHNob3VsZCBiZSBoYW5k bGVkIGJ5IHRoZSBicmlkZ2UgZHJpdmVyIGluIHRoZSBjaGVjawo+ID4+IGNhbGxiYWNrPyBUaGUg Y2FsbGJhY2sgSSdtIGNoYW5naW5nIGlzIGF0dGFjaGVkIHRvIHRoZSBjb25uZWN0b3IsCj4gPj4g d2hpY2ggSSB0aGluayBkb2Vzbid0IGdldCB1c2VkIGlmIHlvdSBoYXZlIGEgYnJpZGdlIGluc3Rl YWQuCj4gPj4gQW5kIHRoaXMgb25seSBjaGVja3MgdGhlIHByZS1yZWdpc3RlcmVkIGRpc3BsYXkg bW9kZXMsIHN1Y2ggYXMKPiA+PiB0aG9zZSBzcGVjaWZpZWQgaW4gc2ltcGxlLXBhbmVsIG9yIEVE SUQuCj4gPj4gCj4gPj4+IFdlIGNvdWxkIGp1c3QgdHJ5IHRvIHNlZSBpZiB0aGUgc2NyZWVuIHBp eGVsIGNsb2NrIGZyZXF1ZW5jeSBpcyBvdXQgb2YKPiA+Pj4gdGhlIHBpeGVsIGNsb2NrIHJhbmdl IHdlIGNhbiBnZW5lcmF0ZSwgYnV0IHRoZW4gd2Ugd2lsbCBsb29wIGJhY2sgb24KPiA+Pj4gaG93 IG11Y2ggb3V0IG9mIHJhbmdlIGlzIGl0IGV4YWN0bHksIGFuZCBpcyBpdCB3aXRoaW4gdGhlIHNj cmVlbgo+ID4+PiB0b2xlcmFuY3kuCj4gPj4+IAo+ID4+PiBXZSBoYXZlIGFuIEFQSSB0byBkZWFs IHdpdGggdGhlIHBhbmVsIHRvbGVyYW5jaWVzIGluIHRoZSBEUk0gcGFuZWwKPiA+Pj4gZnJhbWV3 b3JrLCB3ZSBjYW4gKGFuZCBzaG91bGQpIHVzZSBpdC4KPiA+PiAKPiA+PiBJZiB5b3UgbWVhbiB0 aGUgZ2V0X3RpbWluZ3MgY2FsbGJhY2ssIGl0J3Mgbm90IHZlcnkgdXNlZnVsLiBNb3N0IG9mCj4g Pj4gdGhlIHBhbmVscyBpbiBzaW1wbGUtcGFuZWwgZG8gbm90IHVzZSB0aGUgZGlzcGxheV90aW1p bmdzIHN0cnVjdHVyZSwKPiA+PiBzbyB0aGV5IGRvbid0IHJldHVybiBhbnl0aGluZy4gQW5kIEkg Z2V0IHRoYXQuIFRoZSBmZXcgZGF0YXNoZWV0cwo+ID4+IEkgZm91bmQgZG9uJ3QgbGlzdCBtaW4v bWF4IHRvbGVyYW5jZXMgZm9yIHRoZSBkb3RjbG9jay4KPiA+PiAKPiA+PiBUaGUgb25lcyB0aGF0 IGRvIGhhdmUgdGhlIG1pbi9tYXggdGhlIHNhbWUgYXMgdGhlIHJlY29tbWVuZGVkIHZhbHVlLgo+ ID4+IFRoaXMgbWF5IG9yIG1heSBub3QgYmUgYWNjdXJhdGUuIElJUkMgdGhlIG9uZSBwYW5lbCB0 aGF0IGhhZCB0aGlzCj4gPj4gdGhhdCBJIGRpZCBjaGVjayBkaWRuJ3QgbGlzdCBtaW4vbWF4IHZh bHVlcyBpbiBpdHMgZGF0YXNoZWV0Lgo+ID4+IAo+ID4+PiBJJ20gbm90IHN1cmUgaG93IG90aGVy cyB1c3VhbGx5IGRlYWwgd2l0aCB0aGlzIHRob3VnaC4gSSB0aGluayBJCj4gPj4+IHJlbWVtYmVy IEVyaWMgdGVsbGluZyBtZSB0aGF0IGZvciB0aGUgUlBpIHRoZXkganVzdCBhZGp1c3RlZCB0aGUK PiA+Pj4gdGltaW5ncyBhIGJpdCwgYnV0IHRoZXkgb25seSByZWFsbHkgaGFkIGEgc2luZ2xlIHBh bmVsIHRvIGRlYWwgd2l0aC4KPiA+Pj4gCj4gPj4+IERhbmllbCwgRXJpYywgTGF1cmVudCwgU2Vh bj8gQW55IGlkZWFzPwo+ID4+IAo+ID4+IFllcyEgRmVlZGJhY2sgcGxlYXNlISBCZXR3ZWVuIE1h eGltZSBhbmQgbWUgSSB0aGluayB3ZSBvbmx5IGhhdmUgYQo+ID4+IGxpbWl0ZWQgbnVtYmVyIG9m IHBhbmVscywgd2l0aCBzb21lIG92ZXJsYXAuCgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hh cnQKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1k ZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczov L2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751768AbdB0Id7 (ORCPT ); Mon, 27 Feb 2017 03:33:59 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:33942 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751510AbdB0Ids (ORCPT ); Mon, 27 Feb 2017 03:33:48 -0500 From: Laurent Pinchart To: Sean Paul 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 Date: Mon, 27 Feb 2017 10:26:44 +0200 Message-ID: <2092223.yo6q8ldtYz@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.6-gentoo-r1; KDE/4.14.28; x86_64; ; ) In-Reply-To: <20170223155437.GA24066@art_vandelay> References: <20161124112231.4297-1-wens@csie.org> <7603150.D9x404uVey@avalon> <20170223155437.GA24066@art_vandelay> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sean, On Thursday 23 Feb 2017 10:54:37 Sean Paul wrote: > On Wed, Dec 07, 2016 at 11:48:55AM +0200, Laurent Pinchart wrote: > > 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. There's no such thing as exact frequency when you live in a world that has to comply with the laws of physics :-) This would require an infinitely precise clock, and there's no such thing. > 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'm curious, how so ? What happens to the panel if you feed it a clock outside of that range ? > 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. More than tricky, in the vast majority of cases we don't have that information at all :-/ > > 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