From mboxrd@z Thu Jan 1 00:00:00 1970 From: ville.syrjala@linux.intel.com (Ville =?iso-8859-1?Q?Syrj=E4l=E4?=) Date: Wed, 22 Feb 2017 17:42:30 +0200 Subject: [BUG] hdlcd gets confused about base address In-Reply-To: <20170220185948.GR21222@n2100.armlinux.org.uk> References: <20161118233733.GP1041@n2100.armlinux.org.uk> <20170220121603.GA1132@n2100.armlinux.org.uk> <20170220175331.GC963@e110455-lin.cambridge.arm.com> <20170220180558.GB31595@intel.com> <20170220185948.GR21222@n2100.armlinux.org.uk> Message-ID: <20170222154230.GJ31595@intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Feb 20, 2017 at 06:59:49PM +0000, Russell King - ARM Linux wrote: > On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrj?l? wrote: > > This stuff should be using the clipped coordinates, not the user > > coordinates. And it doesn't look like this guy is even calling the > > clip helper thing. > > > > malidp seems to be calling that thing, but it still doesn't > > manage to program the hw with the right coordinates from what > > I can see. > > > > /me feels a bit like a broken record... > > If you mean drm_plane_helper_check_state(), then... > > $ grep drm_plane_helper_check_state Documentation/gpu/ -r > > So nothing there... but in drivers/gpu/drm/drm_plane_helper.c, there's > the following, and I think this really isn't helping people understand > what's required: > > * This helper library has two parts. The first part has support to implement > * primary plane support on top of the normal CRTC configuration interface. > * Since the legacy ->set_config interface ties the primary plane together with > * the CRTC state this does not allow userspace to disable the primary plane > * itself. To avoid too much duplicated code use > * drm_plane_helper_check_update() which can be used to enforce the same > * restrictions as primary planes had thus. The default primary plane only > * expose XRBG8888 and ARGB8888 as valid pixel formats for the attached > * framebuffer. > * > * Drivers are highly recommended to implement proper support for primary > * planes, and newly merged drivers must not rely upon these transitional > * helpers. > > Which functions are defined as "these transitional helpers" - the above > is rather ambiguous. Is drm_plane_helper_check_state() a "transitional > helper" or is it not? (It probably isn't, but the documentation does not > make that clear.) Nope. And I guess we might want to move it into some atomic code instead. IIRC Daniel even suggested that but I was too lazy to do it at the time. > > It then goes on to: > > * The second part also implements transitional helpers which allow drivers to > > So maybe the second paragraph needs to be moved after this line to > remove the confusion? > > If you find that you're repeating something to many people, it's always > a good idea to re-read the documentation that's supposed to be giving > people guidance. Docs are such a new thing. I've not ever read them through myself TBH. > > Now, when you say that we're supposed to program "clipped coordinates" > maybe you can give a hint what those are and where they come from? > Is that the vaguely documented "clip" parameter in > drm_plane_helper_check_state() ? > > * @clip: integer clipping coordinatesa /** * struct drm_plane_state - mutable plane state ... * @src: clipped source coordinates of the plane (in 16.16) * @dst: clipped destination coordinates of the plane > > If it is, that doesn't really describe it, and neither does the > description of what the function does, nor what it returns: > > * Checks that a desired plane update is valid. Drivers that provide > * their own plane handling rather than helper-provided implementations may > * still wish to call this function to avoid duplication of error checking > * code. > * > * RETURNS: > * Zero if update appears valid, error code on failure > > So, some improvement there could go a long way towards eliminating > some of these issues... > > Atomic modeset is hideously complex... having poor documentation doesn't > help. > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- Ville Syrj?l? Intel OTC From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [BUG] hdlcd gets confused about base address Date: Wed, 22 Feb 2017 17:42:30 +0200 Message-ID: <20170222154230.GJ31595@intel.com> References: <20161118233733.GP1041@n2100.armlinux.org.uk> <20170220121603.GA1132@n2100.armlinux.org.uk> <20170220175331.GC963@e110455-lin.cambridge.arm.com> <20170220180558.GB31595@intel.com> <20170220185948.GR21222@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 79E476E843 for ; Wed, 22 Feb 2017 15:42:35 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170220185948.GR21222@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 Cc: Mali DP Maintainers , Liviu Dudau , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCBGZWIgMjAsIDIwMTcgYXQgMDY6NTk6NDlQTSArMDAwMCwgUnVzc2VsbCBLaW5nIC0g QVJNIExpbnV4IHdyb3RlOgo+IE9uIE1vbiwgRmViIDIwLCAyMDE3IGF0IDA4OjA1OjU4UE0gKzAy MDAsIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPiA+IFRoaXMgc3R1ZmYgc2hvdWxkIGJlIHVzaW5n IHRoZSBjbGlwcGVkIGNvb3JkaW5hdGVzLCBub3QgdGhlIHVzZXIKPiA+IGNvb3JkaW5hdGVzLiBB bmQgaXQgZG9lc24ndCBsb29rIGxpa2UgdGhpcyBndXkgaXMgZXZlbiBjYWxsaW5nIHRoZQo+ID4g Y2xpcCBoZWxwZXIgdGhpbmcuCj4gPiAKPiA+IG1hbGlkcCBzZWVtcyB0byBiZSBjYWxsaW5nIHRo YXQgdGhpbmcsIGJ1dCBpdCBzdGlsbCBkb2Vzbid0Cj4gPiBtYW5hZ2UgdG8gcHJvZ3JhbSB0aGUg aHcgd2l0aCB0aGUgcmlnaHQgY29vcmRpbmF0ZXMgZnJvbSB3aGF0Cj4gPiBJIGNhbiBzZWUuCj4g PiAKPiA+IC9tZSBmZWVscyBhIGJpdCBsaWtlIGEgYnJva2VuIHJlY29yZC4uLgo+IAo+IElmIHlv dSBtZWFuIGRybV9wbGFuZV9oZWxwZXJfY2hlY2tfc3RhdGUoKSwgdGhlbi4uLgo+IAo+ICQgZ3Jl cCBkcm1fcGxhbmVfaGVscGVyX2NoZWNrX3N0YXRlIERvY3VtZW50YXRpb24vZ3B1LyAtcgo+IAo+ IFNvIG5vdGhpbmcgdGhlcmUuLi4gYnV0IGluIGRyaXZlcnMvZ3B1L2RybS9kcm1fcGxhbmVfaGVs cGVyLmMsIHRoZXJlJ3MKPiB0aGUgZm9sbG93aW5nLCBhbmQgSSB0aGluayB0aGlzIHJlYWxseSBp c24ndCBoZWxwaW5nIHBlb3BsZSB1bmRlcnN0YW5kCj4gd2hhdCdzIHJlcXVpcmVkOgo+IAo+ICAq IFRoaXMgaGVscGVyIGxpYnJhcnkgaGFzIHR3byBwYXJ0cy4gVGhlIGZpcnN0IHBhcnQgaGFzIHN1 cHBvcnQgdG8gaW1wbGVtZW50Cj4gICogcHJpbWFyeSBwbGFuZSBzdXBwb3J0IG9uIHRvcCBvZiB0 aGUgbm9ybWFsIENSVEMgY29uZmlndXJhdGlvbiBpbnRlcmZhY2UuCj4gICogU2luY2UgdGhlIGxl Z2FjeSAtPnNldF9jb25maWcgaW50ZXJmYWNlIHRpZXMgdGhlIHByaW1hcnkgcGxhbmUgdG9nZXRo ZXIgd2l0aAo+ICAqIHRoZSBDUlRDIHN0YXRlIHRoaXMgZG9lcyBub3QgYWxsb3cgdXNlcnNwYWNl IHRvIGRpc2FibGUgdGhlIHByaW1hcnkgcGxhbmUKPiAgKiBpdHNlbGYuICBUbyBhdm9pZCB0b28g bXVjaCBkdXBsaWNhdGVkIGNvZGUgdXNlCj4gICogZHJtX3BsYW5lX2hlbHBlcl9jaGVja191cGRh dGUoKSB3aGljaCBjYW4gYmUgdXNlZCB0byBlbmZvcmNlIHRoZSBzYW1lCj4gICogcmVzdHJpY3Rp b25zIGFzIHByaW1hcnkgcGxhbmVzIGhhZCB0aHVzLiBUaGUgZGVmYXVsdCBwcmltYXJ5IHBsYW5l IG9ubHkKPiAgKiBleHBvc2UgWFJCRzg4ODggYW5kIEFSR0I4ODg4IGFzIHZhbGlkIHBpeGVsIGZv cm1hdHMgZm9yIHRoZSBhdHRhY2hlZAo+ICAqIGZyYW1lYnVmZmVyLgo+ICAqCj4gICogRHJpdmVy cyBhcmUgaGlnaGx5IHJlY29tbWVuZGVkIHRvIGltcGxlbWVudCBwcm9wZXIgc3VwcG9ydCBmb3Ig cHJpbWFyeQo+ICAqIHBsYW5lcywgYW5kIG5ld2x5IG1lcmdlZCBkcml2ZXJzIG11c3Qgbm90IHJl bHkgdXBvbiB0aGVzZSB0cmFuc2l0aW9uYWwKPiAgKiBoZWxwZXJzLgo+IAo+IFdoaWNoIGZ1bmN0 aW9ucyBhcmUgZGVmaW5lZCBhcyAidGhlc2UgdHJhbnNpdGlvbmFsIGhlbHBlcnMiIC0gdGhlIGFi b3ZlCj4gaXMgcmF0aGVyIGFtYmlndW91cy4gIElzIGRybV9wbGFuZV9oZWxwZXJfY2hlY2tfc3Rh dGUoKSBhICJ0cmFuc2l0aW9uYWwKPiBoZWxwZXIiIG9yIGlzIGl0IG5vdD8gIChJdCBwcm9iYWJs eSBpc24ndCwgYnV0IHRoZSBkb2N1bWVudGF0aW9uIGRvZXMgbm90Cj4gbWFrZSB0aGF0IGNsZWFy LikKCk5vcGUuIEFuZCBJIGd1ZXNzIHdlIG1pZ2h0IHdhbnQgdG8gbW92ZSBpdCBpbnRvIHNvbWUg YXRvbWljIGNvZGUKaW5zdGVhZC4gSUlSQyBEYW5pZWwgZXZlbiBzdWdnZXN0ZWQgdGhhdCBidXQg SSB3YXMgdG9vIGxhenkgdG8gZG8gaXQgYXQKdGhlIHRpbWUuCgo+IAo+IEl0IHRoZW4gZ29lcyBv biB0bzoKPiAKPiAgKiBUaGUgc2Vjb25kIHBhcnQgYWxzbyBpbXBsZW1lbnRzIHRyYW5zaXRpb25h bCBoZWxwZXJzIHdoaWNoIGFsbG93IGRyaXZlcnMgdG8KPiAKPiBTbyBtYXliZSB0aGUgc2Vjb25k IHBhcmFncmFwaCBuZWVkcyB0byBiZSBtb3ZlZCBhZnRlciB0aGlzIGxpbmUgdG8KPiByZW1vdmUg dGhlIGNvbmZ1c2lvbj8KPiAKPiBJZiB5b3UgZmluZCB0aGF0IHlvdSdyZSByZXBlYXRpbmcgc29t ZXRoaW5nIHRvIG1hbnkgcGVvcGxlLCBpdCdzIGFsd2F5cwo+IGEgZ29vZCBpZGVhIHRvIHJlLXJl YWQgdGhlIGRvY3VtZW50YXRpb24gdGhhdCdzIHN1cHBvc2VkIHRvIGJlIGdpdmluZwo+IHBlb3Bs ZSBndWlkYW5jZS4KCkRvY3MgYXJlIHN1Y2ggYSBuZXcgdGhpbmcuIEkndmUgbm90IGV2ZXIgcmVh ZCB0aGVtIHRocm91Z2ggbXlzZWxmIFRCSC4KCj4gCj4gTm93LCB3aGVuIHlvdSBzYXkgdGhhdCB3 ZSdyZSBzdXBwb3NlZCB0byBwcm9ncmFtICJjbGlwcGVkIGNvb3JkaW5hdGVzIgo+IG1heWJlIHlv dSBjYW4gZ2l2ZSBhIGhpbnQgd2hhdCB0aG9zZSBhcmUgYW5kIHdoZXJlIHRoZXkgY29tZSBmcm9t Pwo+IElzIHRoYXQgdGhlIHZhZ3VlbHkgZG9jdW1lbnRlZCAiY2xpcCIgcGFyYW1ldGVyIGluCj4g ZHJtX3BsYW5lX2hlbHBlcl9jaGVja19zdGF0ZSgpID8KPiAKPiAgKiBAY2xpcDogaW50ZWdlciBj bGlwcGluZyBjb29yZGluYXRlc2EKCi8qKgogKiBzdHJ1Y3QgZHJtX3BsYW5lX3N0YXRlIC0gbXV0 YWJsZSBwbGFuZSBzdGF0ZQogLi4uCiAqIEBzcmM6IGNsaXBwZWQgc291cmNlIGNvb3JkaW5hdGVz IG9mIHRoZSBwbGFuZSAoaW4gMTYuMTYpCiAqIEBkc3Q6IGNsaXBwZWQgZGVzdGluYXRpb24gY29v cmRpbmF0ZXMgb2YgdGhlIHBsYW5lCgo+IAo+IElmIGl0IGlzLCB0aGF0IGRvZXNuJ3QgcmVhbGx5 IGRlc2NyaWJlIGl0LCBhbmQgbmVpdGhlciBkb2VzIHRoZQo+IGRlc2NyaXB0aW9uIG9mIHdoYXQg dGhlIGZ1bmN0aW9uIGRvZXMsIG5vciB3aGF0IGl0IHJldHVybnM6Cj4gCj4gICogQ2hlY2tzIHRo YXQgYSBkZXNpcmVkIHBsYW5lIHVwZGF0ZSBpcyB2YWxpZC4gIERyaXZlcnMgdGhhdCBwcm92aWRl Cj4gICogdGhlaXIgb3duIHBsYW5lIGhhbmRsaW5nIHJhdGhlciB0aGFuIGhlbHBlci1wcm92aWRl ZCBpbXBsZW1lbnRhdGlvbnMgbWF5Cj4gICogc3RpbGwgd2lzaCB0byBjYWxsIHRoaXMgZnVuY3Rp b24gdG8gYXZvaWQgZHVwbGljYXRpb24gb2YgZXJyb3IgY2hlY2tpbmcKPiAgKiBjb2RlLgo+ICAq Cj4gICogUkVUVVJOUzoKPiAgKiBaZXJvIGlmIHVwZGF0ZSBhcHBlYXJzIHZhbGlkLCBlcnJvciBj b2RlIG9uIGZhaWx1cmUKPiAKPiBTbywgc29tZSBpbXByb3ZlbWVudCB0aGVyZSBjb3VsZCBnbyBh IGxvbmcgd2F5IHRvd2FyZHMgZWxpbWluYXRpbmcKPiBzb21lIG9mIHRoZXNlIGlzc3Vlcy4uLgo+ IAo+IEF0b21pYyBtb2Rlc2V0IGlzIGhpZGVvdXNseSBjb21wbGV4Li4uIGhhdmluZyBwb29yIGRv Y3VtZW50YXRpb24gZG9lc24ndAo+IGhlbHAuCj4gCj4gLS0gCj4gUk1LJ3MgUGF0Y2ggc3lzdGVt OiBodHRwOi8vd3d3LmFybWxpbnV4Lm9yZy51ay9kZXZlbG9wZXIvcGF0Y2hlcy8KPiBGVFRDIGJy b2FkYmFuZCBmb3IgMC44bWlsZSBsaW5lOiBjdXJyZW50bHkgYXQgOS42TWJwcyBkb3duIDQwMGti cHMgdXAKPiBhY2NvcmRpbmcgdG8gc3BlZWR0ZXN0Lm5ldC4KCi0tIApWaWxsZSBTeXJqw6Rsw6QK SW50ZWwgT1RDCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpo dHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=