From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:50424 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387404AbeITRF1 (ORCPT ); Thu, 20 Sep 2018 13:05:27 -0400 From: Laurent Pinchart To: Kieran Bingham Cc: linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, Alexandru-Cosmin Gheorghe , David Airlie , open list Subject: Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes Date: Thu, 20 Sep 2018 14:22:38 +0300 Message-ID: <1604918.OXgyMDt7RE@avalon> In-Reply-To: <20180919155700.10342-3-kieran.bingham+renesas@ideasonboard.com> References: <20180919155700.10342-1-kieran.bingham+renesas@ideasonboard.com> <20180919155700.10342-3-kieran.bingham+renesas@ideasonboard.com> 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 Kieran, Thank you for the patch. On Wednesday, 19 September 2018 18:56:59 EEST Kieran Bingham wrote: > If the alpha property is not added to a plane, a default value will be > used, which can result in a non-visible layer if the alpha is > initialised as 0. > > Provide an alpha blend property on all planes. > > Fixes: 161ad653d6c9 ("drm: rcar-du: Use __drm_atomic_helper_plane_reset > instead of copying the logic") > > Signed-off-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 9e07758a755c..72399a19d8a6 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > @@ -783,13 +783,18 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp) > drm_plane_helper_add(&plane->plane, > &rcar_du_plane_helper_funcs); > > + /* > + * The alpha property needs to be initialised on all planes > + * to ensure the correct setting at the output. > + */ > + drm_plane_create_alpha_property(&plane->plane); > + As mentioned in the cover letter, both patches in this series fix the issue at hand. The first patch is more generic as it will fix it for all drivers, while this patch is specific to the R-Car DU driver. It however makes sense to merge it, as it adds alpha support to the primary plane, which can be useful. Once the first patch gets merged, the above comment won't be correct anymore. I wonder whether we shouldn't change the patch description and comment to focus on usage of the alpha property for primary planes, and not on the bug fix. What's your opinion ? > if (type == DRM_PLANE_TYPE_PRIMARY) > continue; > > drm_object_attach_property(&plane->plane.base, > rcdu->props.colorkey, > RCAR_DU_COLORKEY_NONE); > - drm_plane_create_alpha_property(&plane->plane); > drm_plane_create_zpos_property(&plane->plane, 1, 1, 7); > } -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 2/2] drm: rcar-du: Enable alpha property on primary planes Date: Thu, 20 Sep 2018 14:22:38 +0300 Message-ID: <1604918.OXgyMDt7RE@avalon> References: <20180919155700.10342-1-kieran.bingham+renesas@ideasonboard.com> <20180919155700.10342-3-kieran.bingham+renesas@ideasonboard.com> 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 367BF6E5BC for ; Thu, 20 Sep 2018 11:22:27 +0000 (UTC) In-Reply-To: <20180919155700.10342-3-kieran.bingham+renesas@ideasonboard.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Kieran Bingham Cc: linux-renesas-soc@vger.kernel.org, David Airlie , Alexandru-Cosmin Gheorghe , open list , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SGkgS2llcmFuLAoKVGhhbmsgeW91IGZvciB0aGUgcGF0Y2guCgpPbiBXZWRuZXNkYXksIDE5IFNl cHRlbWJlciAyMDE4IDE4OjU2OjU5IEVFU1QgS2llcmFuIEJpbmdoYW0gd3JvdGU6Cj4gSWYgdGhl IGFscGhhIHByb3BlcnR5IGlzIG5vdCBhZGRlZCB0byBhIHBsYW5lLCBhIGRlZmF1bHQgdmFsdWUg d2lsbCBiZQo+IHVzZWQsIHdoaWNoIGNhbiByZXN1bHQgaW4gYSBub24tdmlzaWJsZSBsYXllciBp ZiB0aGUgYWxwaGEgaXMKPiBpbml0aWFsaXNlZCBhcyAwLgo+IAo+IFByb3ZpZGUgYW4gYWxwaGEg YmxlbmQgcHJvcGVydHkgb24gYWxsIHBsYW5lcy4KPiAKPiBGaXhlczogMTYxYWQ2NTNkNmM5ICgi ZHJtOiByY2FyLWR1OiBVc2UgX19kcm1fYXRvbWljX2hlbHBlcl9wbGFuZV9yZXNldAo+IGluc3Rl YWQgb2YgY29weWluZyB0aGUgbG9naWMiKQo+IAo+IFNpZ25lZC1vZmYtYnk6IEtpZXJhbiBCaW5n aGFtIDxraWVyYW4uYmluZ2hhbStyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+Cj4gLS0tCj4gIGRy aXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfcGxhbmUuYyB8IDcgKysrKysrLQo+ICAxIGZp bGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCj4gCj4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfcGxhbmUuYwo+IGIvZHJpdmVycy9n cHUvZHJtL3JjYXItZHUvcmNhcl9kdV9wbGFuZS5jIGluZGV4IDllMDc3NThhNzU1Yy4uNzIzOTlh MTlkOGE2Cj4gMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9w bGFuZS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9wbGFuZS5jCj4g QEAgLTc4MywxMyArNzgzLDE4IEBAIGludCByY2FyX2R1X3BsYW5lc19pbml0KHN0cnVjdCByY2Fy X2R1X2dyb3VwICpyZ3JwKQo+ICAJCWRybV9wbGFuZV9oZWxwZXJfYWRkKCZwbGFuZS0+cGxhbmUs Cj4gIAkJCQkgICAgICZyY2FyX2R1X3BsYW5lX2hlbHBlcl9mdW5jcyk7Cj4gCj4gKwkJLyoKPiAr CQkgKiBUaGUgYWxwaGEgcHJvcGVydHkgbmVlZHMgdG8gYmUgaW5pdGlhbGlzZWQgb24gYWxsIHBs YW5lcwo+ICsJCSAqIHRvIGVuc3VyZSB0aGUgY29ycmVjdCBzZXR0aW5nIGF0IHRoZSBvdXRwdXQu Cj4gKwkJICovCj4gKwkJZHJtX3BsYW5lX2NyZWF0ZV9hbHBoYV9wcm9wZXJ0eSgmcGxhbmUtPnBs YW5lKTsKPiArCgpBcyBtZW50aW9uZWQgaW4gdGhlIGNvdmVyIGxldHRlciwgYm90aCBwYXRjaGVz IGluIHRoaXMgc2VyaWVzIGZpeCB0aGUgaXNzdWUgYXQgCmhhbmQuIFRoZSBmaXJzdCBwYXRjaCBp cyBtb3JlIGdlbmVyaWMgYXMgaXQgd2lsbCBmaXggaXQgZm9yIGFsbCBkcml2ZXJzLCB3aGlsZSAK dGhpcyBwYXRjaCBpcyBzcGVjaWZpYyB0byB0aGUgUi1DYXIgRFUgZHJpdmVyLiBJdCBob3dldmVy IG1ha2VzIHNlbnNlIHRvIG1lcmdlIAppdCwgYXMgaXQgYWRkcyBhbHBoYSBzdXBwb3J0IHRvIHRo ZSBwcmltYXJ5IHBsYW5lLCB3aGljaCBjYW4gYmUgdXNlZnVsLgoKT25jZSB0aGUgZmlyc3QgcGF0 Y2ggZ2V0cyBtZXJnZWQsIHRoZSBhYm92ZSBjb21tZW50IHdvbid0IGJlIGNvcnJlY3QgYW55bW9y ZS4gCkkgd29uZGVyIHdoZXRoZXIgd2Ugc2hvdWxkbid0IGNoYW5nZSB0aGUgcGF0Y2ggZGVzY3Jp cHRpb24gYW5kIGNvbW1lbnQgdG8gCmZvY3VzIG9uIHVzYWdlIG9mIHRoZSBhbHBoYSBwcm9wZXJ0 eSBmb3IgcHJpbWFyeSBwbGFuZXMsIGFuZCBub3Qgb24gdGhlIGJ1ZyAKZml4LiBXaGF0J3MgeW91 ciBvcGluaW9uID8KCj4gIAkJaWYgKHR5cGUgPT0gRFJNX1BMQU5FX1RZUEVfUFJJTUFSWSkKPiAg CQkJY29udGludWU7Cj4gCj4gIAkJZHJtX29iamVjdF9hdHRhY2hfcHJvcGVydHkoJnBsYW5lLT5w bGFuZS5iYXNlLAo+ICAJCQkJCSAgIHJjZHUtPnByb3BzLmNvbG9ya2V5LAo+ICAJCQkJCSAgIFJD QVJfRFVfQ09MT1JLRVlfTk9ORSk7Cj4gLQkJZHJtX3BsYW5lX2NyZWF0ZV9hbHBoYV9wcm9wZXJ0 eSgmcGxhbmUtPnBsYW5lKTsKPiAgCQlkcm1fcGxhbmVfY3JlYXRlX3pwb3NfcHJvcGVydHkoJnBs YW5lLT5wbGFuZSwgMSwgMSwgNyk7Cj4gIAl9CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hh cnQKCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==