From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 01/10] drm/atomic: Add __drm_atomic_helper_plane_reset Date: Thu, 26 Jul 2018 23:20:52 +0300 Message-ID: <2271754.L0ZicdVV5V@avalon> References: <20180726161756.1794-1-alexandru-cosmin.gheorghe@arm.com> <20180726161756.1794-2-alexandru-cosmin.gheorghe@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180726161756.1794-2-alexandru-cosmin.gheorghe@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Alexandru Gheorghe Cc: alexandre.belloni@bootlin.com, airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, thellstrom@vmware.com, krzk@kernel.org, maxime.ripard@bootlin.com, wens@csie.org, kgene@kernel.org, malidp@foss.arm.com, linux-graphics-maintainer@vmware.com, sunpeng.li@amd.com, boris.brezillon@bootlin.com, linux-samsung-soc@vger.kernel.org, nd@arm.com, Tony.Cheng@amd.com, linux-arm-kernel@lists.infradead.org, sw0312.kim@samsung.com, nicolas.ferre@microchip.com, shirish.s@amd.com, kyungmin.park@samsung.com, alexander.deucher@amd.com, christian.koenig@amd.com List-Id: linux-samsung-soc@vger.kernel.org SGkgQWxleGFuZHJ1LAoKVGhhbmsgeW91IGZvciB0aGUgcGF0Y2guCgpPbiBUaHVyc2RheSwgMjYg SnVseSAyMDE4IDE5OjE3OjQ3IEVFU1QgQWxleGFuZHJ1IEdoZW9yZ2hlIHdyb3RlOgo+IFRoZXJl IGFyZSBhIGxvdCBvZiBkcml2ZXJzIHRoYXQgc3ViY2xhc3MgZHJtX3BsYW5lX3N0YXRlLCBhbGwg b2YgdGhlbQo+IGR1cGxpY2F0ZSB0aGUgY29kZSB0aGF0IGxpbmtzIHRvZ2hldGhlciB0aGUgcGxh bmUgd2l0aCBwbGFuZV9zdGF0ZS4KCnMvdG9naGV0aGVyL3RvZ2V0aGVyLwoKPiBPbiB0b3Agb2Yg dGhhdCwgZHJpdmVycyB0aGF0IGVuYWJsZSBjb3JlIHByb3BlcnRpZXMgYWxzbyBoYXZlIHRvCj4g ZHVwbGljYXRlIHRoZSBjb2RlIGZvciBpbml0aWFsaXppbmcgdGhlIHByb3BlcnRpZXMgdG8gdGhl aXIgZGVmYXVsdAo+IHZhbHVlcywgd2hpY2ggaW4gYWxsIGNhc2VzIGFyZSB0aGUgc2FtZSBhcyB0 aGUgZGVmYXVsdHMgZnJvbSBjb3JlLgo+IAo+IENoYW5nZSBzaW5jZSB2MToKPiAtIE1ha2UgaXQg Y29uc2lzdGVudCB3aXRoIHRoZSBvdGhlciBoZWxwZXJzIGFuZCByZXF1aXJlIHRoYXQgYm90aAo+ ICAgcGxhbmUgYW5kIHN0YXRlIG5vdCBiZSBOVUxMLCBzdWdnZXN0ZWQgYnkgQm9yaXMgQnJlemls bG9uIGFuZAo+ICAgUGhpbGlwcCBaYWJlbC4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBBbGV4YW5kcnUg R2hlb3JnaGUgPGFsZXhhbmRydS1jb3NtaW4uZ2hlb3JnaGVAYXJtLmNvbT4KPiAtLS0KPiAgZHJp dmVycy9ncHUvZHJtL2RybV9hdG9taWNfaGVscGVyLmMgfCAzMSArKysrKysrKysrKysrKysrKysr Ky0tLS0tLS0tLQo+ICBpbmNsdWRlL2RybS9kcm1fYXRvbWljX2hlbHBlci5oICAgICB8ICAyICsr Cj4gIDIgZmlsZXMgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlvbnMoLSkKPiAK PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWNfaGVscGVyLmMKPiBiL2Ry aXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljX2hlbHBlci5jIGluZGV4IDg2NmEyY2M3MmVmNi4uN2Y1 YWFmYzViMWEwCj4gMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWNfaGVs cGVyLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pY19oZWxwZXIuYwo+IEBAIC0z NTUyLDYgKzM1NTIsMjYgQEAgdm9pZCBkcm1fYXRvbWljX2hlbHBlcl9jcnRjX2Rlc3Ryb3lfc3Rh dGUoc3RydWN0Cj4gZHJtX2NydGMgKmNydGMsIH0KPiAgRVhQT1JUX1NZTUJPTChkcm1fYXRvbWlj X2hlbHBlcl9jcnRjX2Rlc3Ryb3lfc3RhdGUpOwo+IAo+ICsvKioKPiArICogX19kcm1fYXRvbWlj X2hlbHBlcl9wbGFuZV9yZXNldCAtIHJlc2V0cyBwbGFuZXMgc3RhdGUgdG8gZGVmYXVsdCB2YWx1 ZXMKPiArICogQHBsYW5lOiBwbGFuZSBvYmplY3QsIG11c3Qgbm90IGJlIE5VTEwKPiArICogQHN0 YXRlOiBhdG9taWMgcGxhbmUgc3RhdGUsIG11c3Qgbm90IGJlIE5VTEwKPiArICoKPiArICogSW5p dGlhbGl6ZXMgcGxhbmUgc3RhdGUgdG8gZGVmYXVsdC4gVGhpcyBpcyB1c2VmdWwgZm9yIGRyaXZl cnMgdGhhdAo+IHN1YmNsYXNzCj4gKyAqIHRoZSBwbGFuZSBzdGF0ZS4KPiArICovCj4gK3ZvaWQg X19kcm1fYXRvbWljX2hlbHBlcl9wbGFuZV9yZXNldChzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZSwK PiArCQkJCSAgICAgc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAqc3RhdGUpCj4gK3sKPiArCXN0YXRl LT5wbGFuZSA9IHBsYW5lOwo+ICsJc3RhdGUtPnJvdGF0aW9uID0gRFJNX01PREVfUk9UQVRFXzA7 CgpOaXRwaWNraW5nLCBJJ2Qga2VlcCB0aGUgYmxhbmsgbGluZSBoZXJlIHRvIG1ha2UgdGhlIGNv ZGUgYSBiaXQgZWFzaWVyIHRvIHJlYWQKCj4gKwkvKiBSZXNldCB0aGUgYWxwaGEgdmFsdWUgdG8g ZnVsbHkgb3BhcXVlIGlmIGl0IG1hdHRlcnMgKi8KPiArCWlmIChwbGFuZS0+YWxwaGFfcHJvcGVy dHkpCj4gKwkJc3RhdGUtPmFscGhhID0gcGxhbmUtPmFscGhhX3Byb3BlcnR5LT52YWx1ZXNbMV07 CgpBbmQgaGVyZSB0b28uCgpBcGFydCBmcm9tIHRoYXQsCgpSZXZpZXdlZC1ieTogTGF1cmVudCBQ aW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9hcmQuY29tPgoKPiArCXBsYW5lLT5z dGF0ZSA9IHN0YXRlOwo+ICt9Cj4gK0VYUE9SVF9TWU1CT0woX19kcm1fYXRvbWljX2hlbHBlcl9w bGFuZV9yZXNldCk7Cj4gKwo+ICAvKioKPiAgICogZHJtX2F0b21pY19oZWxwZXJfcGxhbmVfcmVz ZXQgLSBkZWZhdWx0ICZkcm1fcGxhbmVfZnVuY3MucmVzZXQgaG9vayBmb3IKPiBwbGFuZXMgKiBA cGxhbmU6IGRybSBwbGFuZQo+IEBAIC0zNTY2LDE1ICszNTg2LDggQEAgdm9pZCBkcm1fYXRvbWlj X2hlbHBlcl9wbGFuZV9yZXNldChzdHJ1Y3QgZHJtX3BsYW5lCj4gKnBsYW5lKQo+IAo+ICAJa2Zy ZWUocGxhbmUtPnN0YXRlKTsKPiAgCXBsYW5lLT5zdGF0ZSA9IGt6YWxsb2Moc2l6ZW9mKCpwbGFu ZS0+c3RhdGUpLCBHRlBfS0VSTkVMKTsKPiAtCj4gLQlpZiAocGxhbmUtPnN0YXRlKSB7Cj4gLQkJ cGxhbmUtPnN0YXRlLT5wbGFuZSA9IHBsYW5lOwo+IC0JCXBsYW5lLT5zdGF0ZS0+cm90YXRpb24g PSBEUk1fTU9ERV9ST1RBVEVfMDsKPiAtCj4gLQkJLyogUmVzZXQgdGhlIGFscGhhIHZhbHVlIHRv IGZ1bGx5IG9wYXF1ZSBpZiBpdCBtYXR0ZXJzICovCj4gLQkJaWYgKHBsYW5lLT5hbHBoYV9wcm9w ZXJ0eSkKPiAtCQkJcGxhbmUtPnN0YXRlLT5hbHBoYSA9IHBsYW5lLT5hbHBoYV9wcm9wZXJ0eS0+ dmFsdWVzWzFdOwo+IC0JfQo+ICsJaWYgKHBsYW5lLT5zdGF0ZSkKPiArCQlfX2RybV9hdG9taWNf aGVscGVyX3BsYW5lX3Jlc2V0KHBsYW5lLCBwbGFuZS0+c3RhdGUpOwo+ICB9Cj4gIEVYUE9SVF9T WU1CT0woZHJtX2F0b21pY19oZWxwZXJfcGxhbmVfcmVzZXQpOwo+IAo+IGRpZmYgLS1naXQgYS9p bmNsdWRlL2RybS9kcm1fYXRvbWljX2hlbHBlci5oCj4gYi9pbmNsdWRlL2RybS9kcm1fYXRvbWlj X2hlbHBlci5oIGluZGV4IDk5ZTJhNTI5N2M2OS4uZjRjN2VkODc2Yzk3IDEwMDY0NAo+IC0tLSBh L2luY2x1ZGUvZHJtL2RybV9hdG9taWNfaGVscGVyLmgKPiArKysgYi9pbmNsdWRlL2RybS9kcm1f YXRvbWljX2hlbHBlci5oCj4gQEAgLTE1Niw2ICsxNTYsOCBAQCB2b2lkIF9fZHJtX2F0b21pY19o ZWxwZXJfY3J0Y19kZXN0cm95X3N0YXRlKHN0cnVjdAo+IGRybV9jcnRjX3N0YXRlICpzdGF0ZSk7 IHZvaWQgZHJtX2F0b21pY19oZWxwZXJfY3J0Y19kZXN0cm95X3N0YXRlKHN0cnVjdAo+IGRybV9j cnRjICpjcnRjLAo+ICAJCQkJCSAgc3RydWN0IGRybV9jcnRjX3N0YXRlICpzdGF0ZSk7Cj4gCj4g K3ZvaWQgX19kcm1fYXRvbWljX2hlbHBlcl9wbGFuZV9yZXNldChzdHJ1Y3QgZHJtX3BsYW5lICpw bGFuZSwKPiArCQkJCSAgICAgc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAqc3RhdGUpOwo+ICB2b2lk IGRybV9hdG9taWNfaGVscGVyX3BsYW5lX3Jlc2V0KHN0cnVjdCBkcm1fcGxhbmUgKnBsYW5lKTsK PiAgdm9pZCBfX2RybV9hdG9taWNfaGVscGVyX3BsYW5lX2R1cGxpY2F0ZV9zdGF0ZShzdHJ1Y3Qg ZHJtX3BsYW5lICpwbGFuZSwKPiAgCQkJCQkgICAgICAgc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAq c3RhdGUpOwoKLS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgoKCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Thu, 26 Jul 2018 23:20:52 +0300 Subject: [PATCH v2 01/10] drm/atomic: Add __drm_atomic_helper_plane_reset In-Reply-To: <20180726161756.1794-2-alexandru-cosmin.gheorghe@arm.com> References: <20180726161756.1794-1-alexandru-cosmin.gheorghe@arm.com> <20180726161756.1794-2-alexandru-cosmin.gheorghe@arm.com> Message-ID: <2271754.L0ZicdVV5V@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Alexandru, Thank you for the patch. On Thursday, 26 July 2018 19:17:47 EEST Alexandru Gheorghe wrote: > There are a lot of drivers that subclass drm_plane_state, all of them > duplicate the code that links toghether the plane with plane_state. s/toghether/together/ > On top of that, drivers that enable core properties also have to > duplicate the code for initializing the properties to their default > values, which in all cases are the same as the defaults from core. > > Change since v1: > - Make it consistent with the other helpers and require that both > plane and state not be NULL, suggested by Boris Brezillon and > Philipp Zabel. > > Signed-off-by: Alexandru Gheorghe > --- > drivers/gpu/drm/drm_atomic_helper.c | 31 ++++++++++++++++++++--------- > include/drm/drm_atomic_helper.h | 2 ++ > 2 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index 866a2cc72ef6..7f5aafc5b1a0 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3552,6 +3552,26 @@ void drm_atomic_helper_crtc_destroy_state(struct > drm_crtc *crtc, } > EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state); > > +/** > + * __drm_atomic_helper_plane_reset - resets planes state to default values > + * @plane: plane object, must not be NULL > + * @state: atomic plane state, must not be NULL > + * > + * Initializes plane state to default. This is useful for drivers that > subclass > + * the plane state. > + */ > +void __drm_atomic_helper_plane_reset(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + state->plane = plane; > + state->rotation = DRM_MODE_ROTATE_0; Nitpicking, I'd keep the blank line here to make the code a bit easier to read > + /* Reset the alpha value to fully opaque if it matters */ > + if (plane->alpha_property) > + state->alpha = plane->alpha_property->values[1]; And here too. Apart from that, Reviewed-by: Laurent Pinchart > + plane->state = state; > +} > +EXPORT_SYMBOL(__drm_atomic_helper_plane_reset); > + > /** > * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for > planes * @plane: drm plane > @@ -3566,15 +3586,8 @@ void drm_atomic_helper_plane_reset(struct drm_plane > *plane) > > kfree(plane->state); > plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL); > - > - if (plane->state) { > - plane->state->plane = plane; > - plane->state->rotation = DRM_MODE_ROTATE_0; > - > - /* Reset the alpha value to fully opaque if it matters */ > - if (plane->alpha_property) > - plane->state->alpha = plane->alpha_property->values[1]; > - } > + if (plane->state) > + __drm_atomic_helper_plane_reset(plane, plane->state); > } > EXPORT_SYMBOL(drm_atomic_helper_plane_reset); > > diff --git a/include/drm/drm_atomic_helper.h > b/include/drm/drm_atomic_helper.h index 99e2a5297c69..f4c7ed876c97 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -156,6 +156,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct > drm_crtc_state *state); void drm_atomic_helper_crtc_destroy_state(struct > drm_crtc *crtc, > struct drm_crtc_state *state); > > +void __drm_atomic_helper_plane_reset(struct drm_plane *plane, > + struct drm_plane_state *state); > void drm_atomic_helper_plane_reset(struct drm_plane *plane); > void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, > struct drm_plane_state *state); -- Regards, Laurent Pinchart