From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Starkey Subject: Re: [PATCH 6/6] drm: mali-dp: Add writeback connector Date: Tue, 18 Apr 2017 18:35:29 +0100 Message-ID: <20170418173529.GC325@e106950-lin.cambridge.arm.com> References: <1480092544-1725-1-git-send-email-brian.starkey@arm.com> <1480092544-1725-7-git-send-email-brian.starkey@arm.com> <20170414114700.552acc82@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by gabe.freedesktop.org (Postfix) with ESMTP id 050FE6E00B for ; Tue, 18 Apr 2017 17:35:32 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170414114700.552acc82@bbrezillon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Boris Brezillon Cc: linux-media@vger.kernel.org, liviu.dudau@arm.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBBcHIgMTQsIDIwMTcgYXQgMTE6NDc6MDBBTSArMDIwMCwgQm9yaXMgQnJlemlsbG9u IHdyb3RlOgo+T24gRnJpLCAyNSBOb3YgMjAxNiAxNjo0OTowNCArMDAwMAo+QnJpYW4gU3Rhcmtl eSA8YnJpYW4uc3RhcmtleUBhcm0uY29tPiB3cm90ZToKPgo+PiArc3RhdGljIGludAo+PiArbWFs aWRwX213X2VuY29kZXJfYXRvbWljX2NoZWNrKHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlciwK Pj4gKwkJCSAgICAgICBzdHJ1Y3QgZHJtX2NydGNfc3RhdGUgKmNydGNfc3RhdGUsCj4+ICsJCQkg ICAgICAgc3RydWN0IGRybV9jb25uZWN0b3Jfc3RhdGUgKmNvbm5fc3RhdGUpCj4+ICt7Cj4+ICsJ c3RydWN0IG1hbGlkcF9td19jb25uZWN0b3Jfc3RhdGUgKm13X3N0YXRlID0gdG9fbXdfc3RhdGUo Y29ubl9zdGF0ZSk7Cj4+ICsJc3RydWN0IG1hbGlkcF9kcm0gKm1hbGlkcCA9IGVuY29kZXItPmRl di0+ZGV2X3ByaXZhdGU7Cj4+ICsJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmI7Cj4+ICsJaW50 IGksIG5fcGxhbmVzOwo+PiArCj4+ICsJaWYgKCFjb25uX3N0YXRlLT53cml0ZWJhY2tfam9iIHx8 ICFjb25uX3N0YXRlLT53cml0ZWJhY2tfam9iLT5mYikKPj4gKwkJcmV0dXJuIDA7Cj4+ICsKPj4g KwlmYiA9IGNvbm5fc3RhdGUtPndyaXRlYmFja19qb2ItPmZiOwo+PiArCWlmICgoZmItPndpZHRo ICE9IGNydGNfc3RhdGUtPm1vZGUuaGRpc3BsYXkpIHx8Cj4+ICsJICAgIChmYi0+aGVpZ2h0ICE9 IGNydGNfc3RhdGUtPm1vZGUudmRpc3BsYXkpKSB7Cj4+ICsJCURSTV9ERUJVR19LTVMoIkludmFs aWQgZnJhbWVidWZmZXIgc2l6ZSAldXgldVxuIiwKPj4gKwkJCQlmYi0+d2lkdGgsIGZiLT5oZWln aHQpOwo+PiArCQlyZXR1cm4gLUVJTlZBTDsKPj4gKwl9Cj4KPlRoZXNlIGNoZWNrcyBsb29rIHBy ZXR0eSBnZW5lcmljIHRvIG1lLiBTaG91bGRuJ3Qgd2UgaGF2ZSBhIGRlZmF1bHQKPmhlbHBlciBk b2luZyB0aGF0Pwo+CgpZZWFoIG1ha2VzIHNlbnNlLiBUaGVzZSBzaG91bGQgYmUgY29tbW9uIHRv IGV2ZXJ5b25lIHVudGlsCmNyb3BwaW5nL3NjYWxpbmcgc3VwcG9ydCBpcyBhZGRlZC4KCj4+ICsK Pj4gKwltd19zdGF0ZS0+Zm9ybWF0ID0KPj4gKwkJbWFsaWRwX2h3X2dldF9mb3JtYXRfaWQoJm1h bGlkcC0+ZGV2LT5tYXAsIFNFX01FTVdSSVRFLAo+PiArCQkJCQlmYi0+cGl4ZWxfZm9ybWF0KTsK Pj4gKwlpZiAobXdfc3RhdGUtPmZvcm1hdCA9PSBNQUxJRFBfSU5WQUxJRF9GT1JNQVRfSUQpIHsK Pgo+U2FtZSBnb2VzIGhlcmUuIEJ5IGFkZGluZyBhIGZvcm1hdF90eXBlcyB0YWJsZSBzaW1pbGFy IHRvIHdoYXQgaXMKPmV4cG9zZWQgaW4gZHJtX3BsYW5lIFsxXSwgd2UgY291bGQgZG8gdGhpcyBj aGVjayBpbiB0aGUgY29yZS4gVGhlIG9ubHkKPnRoaW5nIGxlZnQgdG8gdGhlIGRyaXZlciBpcyB0 aGUgNENDIC0+IGRyaXZlci1zcGVjaWZpYy1pZCBjb252ZXJzaW9uLgo+CgpZZWFoIGNvdWxkIGRv LCBidXQgZ2l2ZW4gb3VyIGRyaXZlciByZXF1aXJlcyB1cyB0byBydW4gdGhyb3VnaCB0aGUKd2hv bGUgdGFibGUgdG8gZ2V0IHRoZSBIVyBJRCBhbnl3YXkgaXQgc2VlbWVkIGxpa2UgdG90YWxseSB3 YXN0ZWQKZWZmb3J0IHRvIGRvIHRoZSBzYW1lIHRoaW5nIGluIHRoZSBjb3JlLgoKSXQncyBwcm9i YWJseSBhIG5lZ2xpZ2libGUgb3ZlcmhlYWQsIGJ1dCBpdCdzIGFsc28gdW5uZWNlc3NhcnkgZm9y CjEwMCUgb2YgdGhlIGN1cnJlbnQgd3JpdGViYWNrIGltcGxlbWVudGF0aW9ucyA7LSkKCklmIGEg ZGlmZmVyZW50IGRyaXZlciBpcyBpbXBsZW1lbnRlZCBzdWNoIHRoYXQgdGhlIEhXIElEIGxvb2t1 cCBpc24ndAphbiBleGhhdXN0aXZlIGxpc3Qgc2VhcmNoIHRoZW4gd2UgY291bGQgYWRkIGEgaGVs cGVyIGZvciB0aGVtIHRvIHVzZQp3aGljaCBjaGVja3MgdGhlIGJsb2IuCgpDaGVlcnMsCi1Ccmlh bgoKPj4gKwkJc3RydWN0IGRybV9mb3JtYXRfbmFtZV9idWYgZm9ybWF0X25hbWU7Cj4+ICsKPj4g KwkJRFJNX0RFQlVHX0tNUygiSW52YWxpZCBwaXhlbCBmb3JtYXQgJXNcbiIsCj4+ICsJCQkgICAg ICBkcm1fZ2V0X2Zvcm1hdF9uYW1lKGZiLT5waXhlbF9mb3JtYXQsICZmb3JtYXRfbmFtZSkpOwo+ PiArCQlyZXR1cm4gLUVJTlZBTDsKPj4gKwl9Cj4+ICsKPj4gKwluX3BsYW5lcyA9IGRybV9mb3Jt YXRfbnVtX3BsYW5lcyhmYi0+cGl4ZWxfZm9ybWF0KTsKPj4gKwlmb3IgKGkgPSAwOyBpIDwgbl9w bGFuZXM7IGkrKykgewo+PiArCQlzdHJ1Y3QgZHJtX2dlbV9jbWFfb2JqZWN0ICpvYmogPSBkcm1f ZmJfY21hX2dldF9nZW1fb2JqKGZiLCBpKTsKPj4gKwkJaWYgKCFtYWxpZHBfaHdfcGl0Y2hfdmFs aWQobWFsaWRwLT5kZXYsIGZiLT5waXRjaGVzW2ldKSkgewo+PiArCQkJRFJNX0RFQlVHX0tNUygi SW52YWxpZCBwaXRjaCAldSBmb3IgcGxhbmUgJWRcbiIsCj4+ICsJCQkJICAgICAgZmItPnBpdGNo ZXNbaV0sIGkpOwo+PiArCQkJcmV0dXJuIC1FSU5WQUw7Cj4+ICsJCX0KPj4gKwkJbXdfc3RhdGUt PnBpdGNoZXNbaV0gPSBmYi0+cGl0Y2hlc1tpXTsKPj4gKwkJbXdfc3RhdGUtPmFkZHJzW2ldID0g b2JqLT5wYWRkciArIGZiLT5vZmZzZXRzW2ldOwo+PiArCX0KPj4gKwltd19zdGF0ZS0+bl9wbGFu ZXMgPSBuX3BsYW5lczsKPj4gKwo+PiArCXJldHVybiAwOwo+PiArfQo+Cj4KPlsxXWh0dHA6Ly9s eHIuZnJlZS1lbGVjdHJvbnMuY29tL3NvdXJjZS9pbmNsdWRlL2RybS9kcm1fcGxhbmUuaCNMNDgy Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZl bCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from foss.arm.com ([217.140.101.70]:59286 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757370AbdDRRfh (ORCPT ); Tue, 18 Apr 2017 13:35:37 -0400 Date: Tue, 18 Apr 2017 18:35:29 +0100 From: Brian Starkey To: Boris Brezillon Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, liviu.dudau@arm.com, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org Subject: Re: [PATCH 6/6] drm: mali-dp: Add writeback connector Message-ID: <20170418173529.GC325@e106950-lin.cambridge.arm.com> References: <1480092544-1725-1-git-send-email-brian.starkey@arm.com> <1480092544-1725-7-git-send-email-brian.starkey@arm.com> <20170414114700.552acc82@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20170414114700.552acc82@bbrezillon> Sender: linux-media-owner@vger.kernel.org List-ID: On Fri, Apr 14, 2017 at 11:47:00AM +0200, Boris Brezillon wrote: >On Fri, 25 Nov 2016 16:49:04 +0000 >Brian Starkey wrote: > >> +static int >> +malidp_mw_encoder_atomic_check(struct drm_encoder *encoder, >> + struct drm_crtc_state *crtc_state, >> + struct drm_connector_state *conn_state) >> +{ >> + struct malidp_mw_connector_state *mw_state = to_mw_state(conn_state); >> + struct malidp_drm *malidp = encoder->dev->dev_private; >> + struct drm_framebuffer *fb; >> + int i, n_planes; >> + >> + if (!conn_state->writeback_job || !conn_state->writeback_job->fb) >> + return 0; >> + >> + fb = conn_state->writeback_job->fb; >> + if ((fb->width != crtc_state->mode.hdisplay) || >> + (fb->height != crtc_state->mode.vdisplay)) { >> + DRM_DEBUG_KMS("Invalid framebuffer size %ux%u\n", >> + fb->width, fb->height); >> + return -EINVAL; >> + } > >These checks look pretty generic to me. Shouldn't we have a default >helper doing that? > Yeah makes sense. These should be common to everyone until cropping/scaling support is added. >> + >> + mw_state->format = >> + malidp_hw_get_format_id(&malidp->dev->map, SE_MEMWRITE, >> + fb->pixel_format); >> + if (mw_state->format == MALIDP_INVALID_FORMAT_ID) { > >Same goes here. By adding a format_types table similar to what is >exposed in drm_plane [1], we could do this check in the core. The only >thing left to the driver is the 4CC -> driver-specific-id conversion. > Yeah could do, but given our driver requires us to run through the whole table to get the HW ID anyway it seemed like totally wasted effort to do the same thing in the core. It's probably a negligible overhead, but it's also unnecessary for 100% of the current writeback implementations ;-) If a different driver is implemented such that the HW ID lookup isn't an exhaustive list search then we could add a helper for them to use which checks the blob. Cheers, -Brian >> + struct drm_format_name_buf format_name; >> + >> + DRM_DEBUG_KMS("Invalid pixel format %s\n", >> + drm_get_format_name(fb->pixel_format, &format_name)); >> + return -EINVAL; >> + } >> + >> + n_planes = drm_format_num_planes(fb->pixel_format); >> + for (i = 0; i < n_planes; i++) { >> + struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i); >> + if (!malidp_hw_pitch_valid(malidp->dev, fb->pitches[i])) { >> + DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n", >> + fb->pitches[i], i); >> + return -EINVAL; >> + } >> + mw_state->pitches[i] = fb->pitches[i]; >> + mw_state->addrs[i] = obj->paddr + fb->offsets[i]; >> + } >> + mw_state->n_planes = n_planes; >> + >> + return 0; >> +} > > >[1]http://lxr.free-electrons.com/source/include/drm/drm_plane.h#L482