From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:56462 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729709AbeGQSPP (ORCPT ); Tue, 17 Jul 2018 14:15:15 -0400 From: Laurent Pinchart To: kieran.bingham+renesas@ideasonboard.com Cc: linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines Date: Tue, 17 Jul 2018 20:42:04 +0300 Message-ID: <24712162.Y2kuyfjJnU@avalon> In-Reply-To: <08fd7b2c-f725-6018-b96e-72ce62dc21d7@ideasonboard.com> References: <5111021.qdhzlld4I3@avalon> <08fd7b2c-f725-6018-b96e-72ce62dc21d7@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, On Tuesday, 17 July 2018 19:08:44 EEST Kieran Bingham wrote: > On 17/07/18 13:52, Laurent Pinchart wrote: > > 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... I haven't checked, but if you think that's the case, I'll trust you :-) > >>>> + return -EINVAL; > >>>> + } > >>>> + > >>>> + rpf->interlaced = cfg->interlaced; > >>>> + > >>>> rpf->fmtinfo = fmtinfo; > >>>> rpf->format.num_planes = fmtinfo->planes; > >>>> rpf->format.plane_fmt[0].bytesperline = cfg->pitch; > > > > [snip] -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines Date: Tue, 17 Jul 2018 20:42:04 +0300 Message-ID: <24712162.Y2kuyfjJnU@avalon> References: <5111021.qdhzlld4I3@avalon> <08fd7b2c-f725-6018-b96e-72ce62dc21d7@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 [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by gabe.freedesktop.org (Postfix) with ESMTPS id 690BD6E822 for ; Tue, 17 Jul 2018 17:41:33 +0000 (UTC) In-Reply-To: <08fd7b2c-f725-6018-b96e-72ce62dc21d7@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+renesas@ideasonboard.com Cc: linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgS2llcmFuLAoKT24gVHVlc2RheSwgMTcgSnVseSAyMDE4IDE5OjA4OjQ0IEVFU1QgS2llcmFu IEJpbmdoYW0gd3JvdGU6Cj4gT24gMTcvMDcvMTggMTM6NTIsIExhdXJlbnQgUGluY2hhcnQgd3Jv dGU6Cj4gPiBPbiBNb25kYXksIDE2IEp1bHkgMjAxOCAyMToyMTowMCBFRVNUIEtpZXJhbiBCaW5n aGFtIHdyb3RlOgo+ID4+IE9uIDI0LzA1LzE4IDEzOjUxLCBMYXVyZW50IFBpbmNoYXJ0IHdyb3Rl Ogo+ID4+PiBPbiBUaHVyc2RheSwgMyBNYXkgMjAxOCAxNjozNjoyMSBFRVNUIEtpZXJhbiBCaW5n aGFtIHdyb3RlOgo+ID4+Pj4gQ2FsY3VsYXRlIHRoZSB0b3AgYW5kIGJvdHRvbSBmaWVsZHMgZm9y IHRoZSBpbnRlcmxhY2VkIGZyYW1lcyBhbmQKPiA+Pj4+IHV0aWxpc2UgdGhlIGV4dGVuZGVkIGRp c3BsYXkgbGlzdCBjb21tYW5kIGZlYXR1cmUgdG8gaW1wbGVtZW50IHRoZQo+ID4+Pj4gYXV0by1m aWVsZCBvcGVyYXRpb25zLiBUaGlzIGFsbG93cyB0aGUgRFUgdG8gdXBkYXRlIHRoZSBWU1AyIHJl Z2lzdGVycwo+ID4+Pj4gZHluYW1pY2FsbHkgYmFzZWQgdXBvbiB0aGUgY3VycmVudGx5IHByb2Nl c3NpbmcgZmllbGQuCj4gPj4+PiAKPiA+Pj4+IFNpZ25lZC1vZmYtYnk6IEtpZXJhbiBCaW5naGFt IDxraWVyYW4uYmluZ2hhbStyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+Cj4gPj4+PiAKPiA+Pj4+ IC0tLQo+ID4+Pj4gCj4gPj4+PiB2MzoKPiA+Pj4+ICAtIFBhc3MgREwgdGhyb3VnaCBwYXJ0aXRp b24gY2FsbHMgdG8gYWxsb3cgYXV0b2NtZCdzIHRvIGJlIHJldHJpZXZlZAo+ID4+Pj4gIC0gRG9j dW1lbnQgaW50ZXJsYWNlZCBmaWVsZCBpbiBzdHJ1Y3QgdnNwMV9kdV9hdG9taWNfY29uZmlnCj4g Pj4+PiAKPiA+Pj4+IHYyOgo+ID4+Pj4gIC0gZml4IGVycm9uZW91cyBCSVQgdmFsdWUgd2hpY2gg ZW5hYmxlZCBpbnRlcmxhY2VkCj4gPj4+PiAgLSBmaXggZmllbGQgaGFuZGxpbmcgYXQgZnJhbWVf ZW5kIGludGVycnVwdAo+ID4+Pj4gCj4gPj4+PiAtLS0KPiA+Pj4+IAo+ID4+Pj4gIGRyaXZlcnMv bWVkaWEvcGxhdGZvcm0vdnNwMS92c3AxX2RsLmMgICB8IDEwICsrKystCj4gPj4+PiAgZHJpdmVy cy9tZWRpYS9wbGF0Zm9ybS92c3AxL3ZzcDFfZHJtLmMgIHwgMTEgKysrKy0KPiA+Pj4+ICBkcml2 ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNwMV9yZWdzLmggfCAgMSArLQo+ID4+Pj4gIGRyaXZl cnMvbWVkaWEvcGxhdGZvcm0vdnNwMS92c3AxX3JwZi5jICB8IDcxICsrKysrKysrKysrKysrKysr KysrKystLQo+ID4+Pj4gIGRyaXZlcnMvbWVkaWEvcGxhdGZvcm0vdnNwMS92c3AxX3J3cGYuaCB8 ICAxICstCj4gPj4+PiAgaW5jbHVkZS9tZWRpYS92c3AxLmggICAgICAgICAgICAgICAgICAgIHwg IDIgKy0KPiA+Pj4+ICA2IGZpbGVzIGNoYW5nZWQsIDkzIGluc2VydGlvbnMoKyksIDMgZGVsZXRp b25zKC0pCj4gPiAKPiA+IFtzbmlwXQo+ID4gCj4gPj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9t ZWRpYS9wbGF0Zm9ybS92c3AxL3ZzcDFfZHJtLmMKPiA+Pj4+IGIvZHJpdmVycy9tZWRpYS9wbGF0 Zm9ybS92c3AxL3ZzcDFfZHJtLmMgaW5kZXgKPiA+Pj4+IDJjM2RiOGI4YWRjZS4uY2MyOWM5ZDk2 YmI3Cj4gPj4+PiAxMDA2NDQKPiA+Pj4+IC0tLSBhL2RyaXZlcnMvbWVkaWEvcGxhdGZvcm0vdnNw MS92c3AxX2RybS5jCj4gPj4+PiArKysgYi9kcml2ZXJzL21lZGlhL3BsYXRmb3JtL3ZzcDEvdnNw MV9kcm0uYwo+ID4+Pj4gQEAgLTgxMSw2ICs4MTEsMTcgQEAgaW50IHZzcDFfZHVfYXRvbWljX3Vw ZGF0ZShzdHJ1Y3QgZGV2aWNlICpkZXYsCj4gPj4+PiB1bnNpZ25lZCBpbnQgcGlwZV9pbmRleCwK PiA+Pj4+ICAJCXJldHVybiAtRUlOVkFMOwo+ID4+Pj4gIAl9Cj4gPj4+PiAKPiA+Pj4+ICsJaWYg KCEodnNwMV9mZWF0dXJlKHZzcDEsIFZTUDFfSEFTX0VYVF9ETCkpICYmIGNmZy0+aW50ZXJsYWNl ZCkgewo+ID4+PiAKPiA+Pj4gTml0cGlja2luZywgd3JpdGluZyB0aGUgY29uZGl0aW9uIGFzCj4g Pj4+IAo+ID4+PiAJaWYgKGNmZy0+aW50ZXJsYWNlZCAmJiAhKHZzcDFfZmVhdHVyZSh2c3AxLCBW U1AxX0hBU19FWFRfREwpKSkKPiA+PiAKPiA+PiBEb25lLgo+ID4+IAo+ID4+PiB3b3VsZCBtYXRj aCB0aGUgY29tbWVudCBiZXR0ZXIuIFlvdSBjYW4gYWxzbyBkcm9wIHRoZSBwYXJlbnRoZXNlcyBh cm91bmQKPiA+Pj4gdGhlIHZzcDFfZmVhdHVyZSgpIGNhbGwuCj4gPj4+IAo+ID4+Pj4gKwkJLyoK PiA+Pj4+ICsJCSAqIEludGVybGFjZWQgc3VwcG9ydCByZXF1aXJlcyBleHRlbmRlZCBkaXNwbGF5 IGxpc3RzIHRvCj4gPj4+PiArCQkgKiBwcm92aWRlIHRoZSBhdXRvLWZsZCBmZWF0dXJlIHdpdGgg dGhlIERVLgo+ID4+Pj4gKwkJICovCj4gPj4+PiArCQlkZXZfZGJnKHZzcDEtPmRldiwgIkludGVy bGFjZWQgdW5zdXBwb3J0ZWQgb24gdGhpcyBvdXRwdXRcbiIpOwo+ID4+PiAKPiA+Pj4gQ291bGQg d2UgY2F0Y2ggdGhpcyBpbiB0aGUgRFUgZHJpdmVyIHRvIGZhaWwgYXRvbWljIHRlc3QgPwo+ID4+ IAo+ID4+IFVnaCAtIEkgdGhvdWdodCBtb3ZpbmcgdGhlIGNvbmZpZ3VyYXRpb24gdG8gdnNwMV9k dV9zZXR1cF9saWYoKSB3b3VsZAo+ID4+IGdpdmUgdXMgdGhpcywgYnV0IHRoYXQgcmV0dXJuIHZh bHVlIGlzIG5vdCBjaGVja2VkIGluIHRoZSBEVS4KPiA+PiAKPiA+PiBIb3cgY2FuIHdlIGludGVy b2dhdGUgdGhlIFZTUDEgdG8gYXNrIGl0IGlmIGl0IHN1cHBvcnRzIGludGVybGFjZWQgZnJvbQo+ ID4+IHJjYXJfZHVfdnNwX3BsYW5lX2F0b21pY19jaGVjaygpPwo+ID4+IAo+ID4+IFNvbWUgZHVt bXkgY2FsbCB0byB2c3AxX2R1X3NldHVwX2xpZigpIHRvIGNoZWNrIHRoZSByZXR1cm4gdmFsdWUg PyBPcgo+ID4+IHNob3VsZCB3ZSBpbXBsZW1lbnQgYSBob29rIHRvIGNhbGwgdGhyb3VnaCB0byBw ZXJmb3JtIGNoZWNrcyBpbiB0aGUgVlNQMQo+ID4+IERSTSBBUEk/Cj4gPiAKPiA+IFdvdWxkIGl0 IGJlIHBvc3NpYmxlIHRvIGp1c3QgaW5mZXIgdGhhdCBmcm9tIHRoZSBEVSBjb21wYXRpYmxlIHN0 cmluZywKPiA+IHdpdGhvdXQgcXVlcnlpbmcgdGhlIFZTUCBkcml2ZXIgPyBPZiBjb3Vyc2UgdGhh dCdzIGEgYml0IG9mIGEgbGF5ZXJpbmcKPiA+IHZpb2xhdGlvbiwgYnV0IGFzIHdlIGtub3cgd2hh dCB0eXBlIG9mIFZTUCBpbnN0YW5jZSBpcyBwcmVzZW50IGluIGVhY2gKPiA+IFNvQywgc3VjaCBh IHNtYWxsIGhhY2sgd291bGRuJ3QgaHVydCBpbiBteSBvcGluaW9uLiBJZiB0aGUgbmVlZCBhcmlz ZXMKPiA+IGxhdGVyIHdlIGNhbiBpbnRyb2R1Y2UgYW4gQVBJIHRvIHF1ZXJ5IHRoZSBpbmZvcm1h dGlvbiBmcm9tIHRoZSBWU1AKPiA+IGRyaXZlci4KPiAKPiBJJ20gbm90IHN1cmUgd2hhdCB0aGVy ZSBpcyB0byBtYXRjaCBvbiBjdXJyZW50bHkuCj4gCj4gSSB0aG91Z2h0IHRoYXQgd2UgaGFkIHJl c3RyaWN0aW9ucyBvbiB3aGljaCBkaXNwbGF5IHBpcGVsaW5lcyBzdXBwb3J0ZWQKPiBpbnRlcmxh Y2VkLiAoaS5lLiBEMy9FMyBtaWdodCBub3QpIC0gYnV0IHRoZXkgc2VlbSB0byBzdXBwb3J0IGV4 dGVuZGVkCj4gZGlzcGxheSBsaXN0cyAuLi4KPiAKPiBTbyBpc24ndCBpdCB0aGUgY2FzZSB0aGF0 IGFueSBwaXBlbGluZSB3aGljaCB3ZSBjb25uZWN0IHRvIERSTSBzdXBwb3J0cwo+IGludGVybGFj ZWQ/IChjdXJyZW50bHkpIC0gd2UgY2FuJ3QgLyBkb24ndCBwaHlzaWNhbGx5IGNvbm5lY3Qgb3Ro ZXIgVlNQCj4gZW50aXRpZXMgdG8gdGhlIERSTSBwaXBlcy4uLgoKSSBoYXZlbid0IGNoZWNrZWQs IGJ1dCBpZiB5b3UgdGhpbmsgdGhhdCdzIHRoZSBjYXNlLCBJJ2xsIHRydXN0IHlvdSA6LSkKCj4g Pj4+PiArCQlyZXR1cm4gLUVJTlZBTDsKPiA+Pj4+ICsJfQo+ID4+Pj4gKwo+ID4+Pj4gKwlycGYt PmludGVybGFjZWQgPSBjZmctPmludGVybGFjZWQ7Cj4gPj4+PiArCj4gPj4+PiAgCXJwZi0+Zm10 aW5mbyA9IGZtdGluZm87Cj4gPj4+PiAgCXJwZi0+Zm9ybWF0Lm51bV9wbGFuZXMgPSBmbXRpbmZv LT5wbGFuZXM7Cj4gPj4+PiAgCXJwZi0+Zm9ybWF0LnBsYW5lX2ZtdFswXS5ieXRlc3BlcmxpbmUg PSBjZmctPnBpdGNoOwo+ID4gCj4gPiBbc25pcF0KCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5j aGFydAoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpk cmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK