From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from perceval.ideasonboard.com ([213.167.242.64]:56136 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729863AbeGQQmK (ORCPT ); Tue, 17 Jul 2018 12:42:10 -0400 Reply-To: kieran.bingham+renesas@ideasonboard.com Subject: Re: [PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines To: Laurent Pinchart Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org References: <2663986.uvcnutGSNp@avalon> <5111021.qdhzlld4I3@avalon> From: Kieran Bingham Message-ID: <08fd7b2c-f725-6018-b96e-72ce62dc21d7@ideasonboard.com> Date: Tue, 17 Jul 2018 17:08:44 +0100 MIME-Version: 1.0 In-Reply-To: <5111021.qdhzlld4I3@avalon> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, On 17/07/18 13:52, Laurent Pinchart wrote: > Hi Kieran, > > On Monday, 16 July 2018 21:21:00 EEST Kieran Bingham wrote: >> On 24/05/18 13:51, Laurent Pinchart wrote: >>> On Thursday, 3 May 2018 16:36:21 EEST Kieran Bingham wrote: >>>> Calculate the top and bottom fields for the interlaced frames and >>>> utilise the extended display list command feature to implement the >>>> auto-field operations. This allows the DU to update the VSP2 registers >>>> dynamically based upon the currently processing field. >>>> >>>> Signed-off-by: Kieran Bingham >>>> >>>> --- >>>> >>>> v3: >>>> - Pass DL through partition calls to allow autocmd's to be retrieved >>>> - Document interlaced field in struct vsp1_du_atomic_config >>>> >>>> v2: >>>> - fix erroneous BIT value which enabled interlaced >>>> - fix field handling at frame_end interrupt >>>> >>>> --- >>>> >>>> drivers/media/platform/vsp1/vsp1_dl.c | 10 ++++- >>>> drivers/media/platform/vsp1/vsp1_drm.c | 11 ++++- >>>> drivers/media/platform/vsp1/vsp1_regs.h | 1 +- >>>> drivers/media/platform/vsp1/vsp1_rpf.c | 71 ++++++++++++++++++++++++-- >>>> drivers/media/platform/vsp1/vsp1_rwpf.h | 1 +- >>>> include/media/vsp1.h | 2 +- >>>> 6 files changed, 93 insertions(+), 3 deletions(-) > > [snip] > >>>> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c >>>> b/drivers/media/platform/vsp1/vsp1_drm.c index 2c3db8b8adce..cc29c9d96bb7 >>>> 100644 >>>> --- a/drivers/media/platform/vsp1/vsp1_drm.c >>>> +++ b/drivers/media/platform/vsp1/vsp1_drm.c >>>> @@ -811,6 +811,17 @@ int vsp1_du_atomic_update(struct device *dev, >>>> unsigned >>>> int pipe_index, return -EINVAL; >>>> >>>> } >>>> >>>> + if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) { >>> >>> Nitpicking, writing the condition as >>> >>> if (cfg->interlaced && !(vsp1_feature(vsp1, VSP1_HAS_EXT_DL))) >> >> Done. >> >>> would match the comment better. You can also drop the parentheses around >>> the vsp1_feature() call. >>> >>>> + /* >>>> + * Interlaced support requires extended display lists to >>>> + * provide the auto-fld feature with the DU. >>>> + */ >>>> + dev_dbg(vsp1->dev, "Interlaced unsupported on this output\n"); >>> >>> Could we catch this in the DU driver to fail atomic test ? >> >> Ugh - I thought moving the configuration to vsp1_du_setup_lif() would >> give us this, but that return value is not checked in the DU. >> >> How can we interogate the VSP1 to ask it if it supports interlaced from >> rcar_du_vsp_plane_atomic_check()? >> >> >> Some dummy call to vsp1_du_setup_lif() to check the return value ? Or >> should we implement a hook to call through to perform checks in the VSP1 >> DRM API? > > Would it be possible to just infer that from the DU compatible string, without > querying the VSP driver ? Of course that's a bit of a layering violation, but > as we know what type of VSP instance is present in each SoC, such a small hack > wouldn't hurt in my opinion. If the need arises later we can introduce an API > to query the information from the VSP driver. I'm not sure what there is to match on currently. I thought that we had restrictions on which display pipelines supported interlaced. (i.e. D3/E3 might not) - but they seem to support extended display lists ... So isn't it the case that any pipeline which we connect to DRM supports interlaced? (currently) - we can't / don't physically connect other VSP entities to the DRM pipes... > >>>> + return -EINVAL; >>>> + } >>>> + >>>> + rpf->interlaced = cfg->interlaced; >>>> + >>>> rpf->fmtinfo = fmtinfo; >>>> rpf->format.num_planes = fmtinfo->planes; >>>> rpf->format.plane_fmt[0].bytesperline = cfg->pitch; > > [snip] From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kieran Bingham Subject: Re: [PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines Date: Tue, 17 Jul 2018 17:08:44 +0100 Message-ID: <08fd7b2c-f725-6018-b96e-72ce62dc21d7@ideasonboard.com> References: <2663986.uvcnutGSNp@avalon> <5111021.qdhzlld4I3@avalon> Reply-To: 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 9AC586E4EC for ; Tue, 17 Jul 2018 16:08:48 +0000 (UTC) In-Reply-To: <5111021.qdhzlld4I3@avalon> Content-Language: en-GB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgTGF1cmVudCwKCk9uIDE3LzA3LzE4IDEzOjUyLCBMYXVyZW50IFBpbmNoYXJ0IHdyb3RlOgo+ IEhpIEtpZXJhbiwKPiAKPiBPbiBNb25kYXksIDE2IEp1bHkgMjAxOCAyMToyMTowMCBFRVNUIEtp ZXJhbiBCaW5naGFtIHdyb3RlOgo+PiBPbiAyNC8wNS8xOCAxMzo1MSwgTGF1cmVudCBQaW5jaGFy dCB3cm90ZToKPj4+IE9uIFRodXJzZGF5LCAzIE1heSAyMDE4IDE2OjM2OjIxIEVFU1QgS2llcmFu IEJpbmdoYW0gd3JvdGU6Cj4+Pj4gQ2FsY3VsYXRlIHRoZSB0b3AgYW5kIGJvdHRvbSBmaWVsZHMg Zm9yIHRoZSBpbnRlcmxhY2VkIGZyYW1lcyBhbmQKPj4+PiB1dGlsaXNlIHRoZSBleHRlbmRlZCBk aXNwbGF5IGxpc3QgY29tbWFuZCBmZWF0dXJlIHRvIGltcGxlbWVudCB0aGUKPj4+PiBhdXRvLWZp ZWxkIG9wZXJhdGlvbnMuIFRoaXMgYWxsb3dzIHRoZSBEVSB0byB1cGRhdGUgdGhlIFZTUDIgcmVn aXN0ZXJzCj4+Pj4gZHluYW1pY2FsbHkgYmFzZWQgdXBvbiB0aGUgY3VycmVudGx5IHByb2Nlc3Np bmcgZmllbGQuCj4+Pj4KPj4+PiBTaWduZWQtb2ZmLWJ5OiBLaWVyYW4gQmluZ2hhbSA8a2llcmFu LmJpbmdoYW0rcmVuZXNhc0BpZGVhc29uYm9hcmQuY29tPgo+Pj4+Cj4+Pj4gLS0tCj4+Pj4KPj4+ PiB2MzoKPj4+PiAgLSBQYXNzIERMIHRocm91Z2ggcGFydGl0aW9uIGNhbGxzIHRvIGFsbG93IGF1 dG9jbWQncyB0byBiZSByZXRyaWV2ZWQKPj4+PiAgLSBEb2N1bWVudCBpbnRlcmxhY2VkIGZpZWxk IGluIHN0cnVjdCB2c3AxX2R1X2F0b21pY19jb25maWcKPj4+Pgo+Pj4+IHYyOgo+Pj4+ICAtIGZp eCBlcnJvbmVvdXMgQklUIHZhbHVlIHdoaWNoIGVuYWJsZWQgaW50ZXJsYWNlZAo+Pj4+ICAtIGZp eCBmaWVsZCBoYW5kbGluZyBhdCBmcmFtZV9lbmQgaW50ZXJydXB0Cj4+Pj4KPj4+PiAtLS0KPj4+ Pgo+Pj4+ICBkcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNwMV9kbC5jICAgfCAxMCArKysr LQo+Pj4+ICBkcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNwMV9kcm0uYyAgfCAxMSArKysr LQo+Pj4+ICBkcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNwMV9yZWdzLmggfCAgMSArLQo+ Pj4+ICBkcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNwMV9ycGYuYyAgfCA3MSArKysrKysr KysrKysrKysrKysrKysrKystLQo+Pj4+ICBkcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNw MV9yd3BmLmggfCAgMSArLQo+Pj4+ICBpbmNsdWRlL21lZGlhL3ZzcDEuaCAgICAgICAgICAgICAg ICAgICAgfCAgMiArLQo+Pj4+ICA2IGZpbGVzIGNoYW5nZWQsIDkzIGluc2VydGlvbnMoKyksIDMg ZGVsZXRpb25zKC0pCj4gCj4gW3NuaXBdCj4gCj4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbWVk aWEvcGxhdGZvcm0vdnNwMS92c3AxX2RybS5jCj4+Pj4gYi9kcml2ZXJzL21lZGlhL3BsYXRmb3Jt L3ZzcDEvdnNwMV9kcm0uYyBpbmRleCAyYzNkYjhiOGFkY2UuLmNjMjljOWQ5NmJiNwo+Pj4+IDEw MDY0NAo+Pj4+IC0tLSBhL2RyaXZlcnMvbWVkaWEvcGxhdGZvcm0vdnNwMS92c3AxX2RybS5jCj4+ Pj4gKysrIGIvZHJpdmVycy9tZWRpYS9wbGF0Zm9ybS92c3AxL3ZzcDFfZHJtLmMKPj4+PiBAQCAt ODExLDYgKzgxMSwxNyBAQCBpbnQgdnNwMV9kdV9hdG9taWNfdXBkYXRlKHN0cnVjdCBkZXZpY2Ug KmRldiwKPj4+PiB1bnNpZ25lZAo+Pj4+IGludCBwaXBlX2luZGV4LCByZXR1cm4gLUVJTlZBTDsK Pj4+Pgo+Pj4+ICAJfQo+Pj4+Cj4+Pj4gKwlpZiAoISh2c3AxX2ZlYXR1cmUodnNwMSwgVlNQMV9I QVNfRVhUX0RMKSkgJiYgY2ZnLT5pbnRlcmxhY2VkKSB7Cj4+Pgo+Pj4gTml0cGlja2luZywgd3Jp dGluZyB0aGUgY29uZGl0aW9uIGFzCj4+Pgo+Pj4gCWlmIChjZmctPmludGVybGFjZWQgJiYgISh2 c3AxX2ZlYXR1cmUodnNwMSwgVlNQMV9IQVNfRVhUX0RMKSkpCj4+Cj4+IERvbmUuCj4+Cj4+PiB3 b3VsZCBtYXRjaCB0aGUgY29tbWVudCBiZXR0ZXIuIFlvdSBjYW4gYWxzbyBkcm9wIHRoZSBwYXJl bnRoZXNlcyBhcm91bmQKPj4+IHRoZSB2c3AxX2ZlYXR1cmUoKSBjYWxsLgo+Pj4KPj4+PiArCQkv Kgo+Pj4+ICsJCSAqIEludGVybGFjZWQgc3VwcG9ydCByZXF1aXJlcyBleHRlbmRlZCBkaXNwbGF5 IGxpc3RzIHRvCj4+Pj4gKwkJICogcHJvdmlkZSB0aGUgYXV0by1mbGQgZmVhdHVyZSB3aXRoIHRo ZSBEVS4KPj4+PiArCQkgKi8KPj4+PiArCQlkZXZfZGJnKHZzcDEtPmRldiwgIkludGVybGFjZWQg dW5zdXBwb3J0ZWQgb24gdGhpcyBvdXRwdXRcbiIpOwo+Pj4KPj4+IENvdWxkIHdlIGNhdGNoIHRo aXMgaW4gdGhlIERVIGRyaXZlciB0byBmYWlsIGF0b21pYyB0ZXN0ID8KPj4KPj4gVWdoIC0gSSB0 aG91Z2h0IG1vdmluZyB0aGUgY29uZmlndXJhdGlvbiB0byB2c3AxX2R1X3NldHVwX2xpZigpIHdv dWxkCj4+IGdpdmUgdXMgdGhpcywgYnV0IHRoYXQgcmV0dXJuIHZhbHVlIGlzIG5vdCBjaGVja2Vk IGluIHRoZSBEVS4KPj4KPj4gSG93IGNhbiB3ZSBpbnRlcm9nYXRlIHRoZSBWU1AxIHRvIGFzayBp dCBpZiBpdCBzdXBwb3J0cyBpbnRlcmxhY2VkIGZyb20KPj4gcmNhcl9kdV92c3BfcGxhbmVfYXRv bWljX2NoZWNrKCk/Cj4+Cj4+Cj4+IFNvbWUgZHVtbXkgY2FsbCB0byB2c3AxX2R1X3NldHVwX2xp ZigpIHRvIGNoZWNrIHRoZSByZXR1cm4gdmFsdWUgPyBPcgo+PiBzaG91bGQgd2UgaW1wbGVtZW50 IGEgaG9vayB0byBjYWxsIHRocm91Z2ggdG8gcGVyZm9ybSBjaGVja3MgaW4gdGhlIFZTUDEKPj4g RFJNIEFQST8KPiAKPiBXb3VsZCBpdCBiZSBwb3NzaWJsZSB0byBqdXN0IGluZmVyIHRoYXQgZnJv bSB0aGUgRFUgY29tcGF0aWJsZSBzdHJpbmcsIHdpdGhvdXQgCj4gcXVlcnlpbmcgdGhlIFZTUCBk cml2ZXIgPyBPZiBjb3Vyc2UgdGhhdCdzIGEgYml0IG9mIGEgbGF5ZXJpbmcgdmlvbGF0aW9uLCBi dXQgCj4gYXMgd2Uga25vdyB3aGF0IHR5cGUgb2YgVlNQIGluc3RhbmNlIGlzIHByZXNlbnQgaW4g ZWFjaCBTb0MsIHN1Y2ggYSBzbWFsbCBoYWNrIAo+IHdvdWxkbid0IGh1cnQgaW4gbXkgb3Bpbmlv bi4gSWYgdGhlIG5lZWQgYXJpc2VzIGxhdGVyIHdlIGNhbiBpbnRyb2R1Y2UgYW4gQVBJIAo+IHRv IHF1ZXJ5IHRoZSBpbmZvcm1hdGlvbiBmcm9tIHRoZSBWU1AgZHJpdmVyLgoKSSdtIG5vdCBzdXJl IHdoYXQgdGhlcmUgaXMgdG8gbWF0Y2ggb24gY3VycmVudGx5LgoKSSB0aG91Z2h0IHRoYXQgd2Ug aGFkIHJlc3RyaWN0aW9ucyBvbiB3aGljaCBkaXNwbGF5IHBpcGVsaW5lcyBzdXBwb3J0ZWQKaW50 ZXJsYWNlZC4gKGkuZS4gRDMvRTMgbWlnaHQgbm90KSAtIGJ1dCB0aGV5IHNlZW0gdG8gc3VwcG9y dCBleHRlbmRlZApkaXNwbGF5IGxpc3RzIC4uLgoKU28gaXNuJ3QgaXQgdGhlIGNhc2UgdGhhdCBh bnkgcGlwZWxpbmUgd2hpY2ggd2UgY29ubmVjdCB0byBEUk0gc3VwcG9ydHMKaW50ZXJsYWNlZD8g KGN1cnJlbnRseSkgLSB3ZSBjYW4ndCAvIGRvbid0IHBoeXNpY2FsbHkgY29ubmVjdCBvdGhlciBW U1AKZW50aXRpZXMgdG8gdGhlIERSTSBwaXBlcy4uLgoKCj4gCj4+Pj4gKwkJcmV0dXJuIC1FSU5W QUw7Cj4+Pj4gKwl9Cj4+Pj4gKwo+Pj4+ICsJcnBmLT5pbnRlcmxhY2VkID0gY2ZnLT5pbnRlcmxh Y2VkOwo+Pj4+ICsKPj4+PiAgCXJwZi0+Zm10aW5mbyA9IGZtdGluZm87Cj4+Pj4gIAlycGYtPmZv cm1hdC5udW1fcGxhbmVzID0gZm10aW5mby0+cGxhbmVzOwo+Pj4+ICAJcnBmLT5mb3JtYXQucGxh bmVfZm10WzBdLmJ5dGVzcGVybGluZSA9IGNmZy0+cGl0Y2g7Cj4gCj4gW3NuaXBdCgoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxp bmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==