From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from perceval.ideasonboard.com ([213.167.242.64]:59594 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754119AbeDZUgf (ORCPT ); Thu, 26 Apr 2018 16:36:35 -0400 From: Laurent Pinchart To: Kieran Bingham Cc: linux-renesas-soc@vger.kernel.org, David Airlie , "open list:DRM DRIVERS FOR RENESAS" , open list Subject: Re: [PATCH 06/17] drm: rcar-du: Allow DU groups to work with hardware indexing Date: Thu, 26 Apr 2018 23:36:48 +0300 Message-ID: <10312647.9oa0HbEkDa@avalon> In-Reply-To: <20180426165346.494-7-kieran.bingham+renesas@ideasonboard.com> References: <20180426165346.494-1-kieran.bingham+renesas@ideasonboard.com> <20180426165346.494-7-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 Thursday, 26 April 2018 19:53:35 EEST Kieran Bingham wrote: > The group objects assume linear indexing, and more so always assume that > channel 0 of any active group is used. > > Now that the CRTC objects support non-linear indexing, adapt the groups > to remove assumptions that channel 0 is utilised in each group by using > the channel mask provided in the device structures. > > Finally ensure that the RGB routing is determined from the index of the > CRTC object (which represents the hardware DU channel index). > > Signed-off-by: Kieran Bingham > --- > drivers/gpu/drm/rcar-du/rcar_du_group.c | 14 +++++++++----- > drivers/gpu/drm/rcar-du/rcar_du_group.h | 2 ++ > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 5 ++++- > 3 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c > b/drivers/gpu/drm/rcar-du/rcar_du_group.c index eead202c95c7..c52091fe02ba > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -46,9 +46,12 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32 > reg, u32 data) > > static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp) > { > - u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP; > + u32 defr6 = DEFR6_CODE; > > - if (rgrp->num_crtcs > 1) > + if (rgrp->channel_mask & BIT(0)) > + defr6 |= DEFR6_ODPM02_DISP; > + > + if (rgrp->channel_mask & BIT(1)) > defr6 |= DEFR6_ODPM12_DISP; So much cleaner with the channels mask, I like this. > rcar_du_group_write(rgrp, DEFR6, defr6); > @@ -80,10 +83,11 @@ static void rcar_du_group_setup_defr8(struct > rcar_du_group *rgrp) * On Gen3 VSPD routing can't be configured, but DPAD > routing > * needs to be set despite having a single option available. > */ > - u32 crtc = ffs(possible_crtcs) - 1; > + unsigned int rgb_crtc = ffs(possible_crtcs) - 1; > + struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc]; > > - if (crtc / 2 == rgrp->index) > - defr8 |= DEFR8_DRGBS_DU(crtc); > + if (crtc->index / 2 == rgrp->index) > + defr8 |= DEFR8_DRGBS_DU(crtc->index); > } > > rcar_du_group_write(rgrp, DEFR8, defr8); > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h > b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 5e3adc6b31b5..d29a68e006a7 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h > @@ -25,6 +25,7 @@ struct rcar_du_device; > * @dev: the DU device > * @mmio_offset: registers offset in the device memory map > * @index: group index > + * @channel_mask: bitmask of populated DU channels in this group > * @num_crtcs: number of CRTCs in this group (1 or 2) > * @use_count: number of users of the group (rcar_du_group_(get|put)) > * @used_crtcs: number of CRTCs currently in use > @@ -39,6 +40,7 @@ struct rcar_du_group { > unsigned int mmio_offset; > unsigned int index; > > + unsigned int channel_mask; Depending on how you like my suggestion in patch 05/17, this might be better called channels_mask. > unsigned int num_crtcs; > unsigned int use_count; > unsigned int used_crtcs; > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 19a445fbc879..45fb554fd3c7 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -597,7 +597,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > rgrp->dev = rcdu; > rgrp->mmio_offset = mmio_offsets[i]; > rgrp->index = i; > - rgrp->num_crtcs = min(rcdu->num_crtcs - 2 * i, 2U); > + /* Extract the channel mask for this group only */ s/only/only./ > + rgrp->channel_mask = (rcdu->info->channel_mask >> (2 * i)) > + & GENMASK(1, 0); > + rgrp->num_crtcs = hweight8(rgrp->channel_mask); You could optimize this by computing it as rgrp->num_crtcs = (rgrp->channel_mask >> 1) | (rgrp->channel_mask & 1); as you know that only two bits at most can be set. Up to you. > /* > * If we have more than one CRTCs in this group pre-associate With those small issues fixed, Reviewed-by: Laurent Pinchart -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 06/17] drm: rcar-du: Allow DU groups to work with hardware indexing Date: Thu, 26 Apr 2018 23:36:48 +0300 Message-ID: <10312647.9oa0HbEkDa@avalon> References: <20180426165346.494-1-kieran.bingham+renesas@ideasonboard.com> <20180426165346.494-7-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 [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6A8DB6E785 for ; Thu, 26 Apr 2018 20:36:35 +0000 (UTC) In-Reply-To: <20180426165346.494-7-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 , open list , "open list:DRM DRIVERS FOR RENESAS" List-Id: dri-devel@lists.freedesktop.org SGkgS2llcmFuLAoKVGhhbmsgeW91IGZvciB0aGUgcGF0Y2guCgpPbiBUaHVyc2RheSwgMjYgQXBy aWwgMjAxOCAxOTo1MzozNSBFRVNUIEtpZXJhbiBCaW5naGFtIHdyb3RlOgo+IFRoZSBncm91cCBv YmplY3RzIGFzc3VtZSBsaW5lYXIgaW5kZXhpbmcsIGFuZCBtb3JlIHNvIGFsd2F5cyBhc3N1bWUg dGhhdAo+IGNoYW5uZWwgMCBvZiBhbnkgYWN0aXZlIGdyb3VwIGlzIHVzZWQuCj4gCj4gTm93IHRo YXQgdGhlIENSVEMgb2JqZWN0cyBzdXBwb3J0IG5vbi1saW5lYXIgaW5kZXhpbmcsIGFkYXB0IHRo ZSBncm91cHMKPiB0byByZW1vdmUgYXNzdW1wdGlvbnMgdGhhdCBjaGFubmVsIDAgaXMgdXRpbGlz ZWQgaW4gZWFjaCBncm91cCBieSB1c2luZwo+IHRoZSBjaGFubmVsIG1hc2sgcHJvdmlkZWQgaW4g dGhlIGRldmljZSBzdHJ1Y3R1cmVzLgo+IAo+IEZpbmFsbHkgZW5zdXJlIHRoYXQgdGhlIFJHQiBy b3V0aW5nIGlzIGRldGVybWluZWQgZnJvbSB0aGUgaW5kZXggb2YgdGhlCj4gQ1JUQyBvYmplY3Qg KHdoaWNoIHJlcHJlc2VudHMgdGhlIGhhcmR3YXJlIERVIGNoYW5uZWwgaW5kZXgpLgo+IAo+IFNp Z25lZC1vZmYtYnk6IEtpZXJhbiBCaW5naGFtIDxraWVyYW4uYmluZ2hhbStyZW5lc2FzQGlkZWFz b25ib2FyZC5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfZ3Jv dXAuYyB8IDE0ICsrKysrKysrKy0tLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJf ZHVfZ3JvdXAuaCB8ICAyICsrCj4gIGRyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfa21z LmMgICB8ICA1ICsrKystCj4gIDMgZmlsZXMgY2hhbmdlZCwgMTUgaW5zZXJ0aW9ucygrKSwgNiBk ZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNh cl9kdV9ncm91cC5jCj4gYi9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2dyb3VwLmMg aW5kZXggZWVhZDIwMmM5NWM3Li5jNTIwOTFmZTAyYmEKPiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2dyb3VwLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v cmNhci1kdS9yY2FyX2R1X2dyb3VwLmMKPiBAQCAtNDYsOSArNDYsMTIgQEAgdm9pZCByY2FyX2R1 X2dyb3VwX3dyaXRlKHN0cnVjdCByY2FyX2R1X2dyb3VwICpyZ3JwLCB1MzIKPiByZWcsIHUzMiBk YXRhKQo+IAo+ICBzdGF0aWMgdm9pZCByY2FyX2R1X2dyb3VwX3NldHVwX3BpbnMoc3RydWN0IHJj YXJfZHVfZ3JvdXAgKnJncnApCj4gIHsKPiAtCXUzMiBkZWZyNiA9IERFRlI2X0NPREUgfCBERUZS Nl9PRFBNMDJfRElTUDsKPiArCXUzMiBkZWZyNiA9IERFRlI2X0NPREU7Cj4gCj4gLQlpZiAocmdy cC0+bnVtX2NydGNzID4gMSkKPiArCWlmIChyZ3JwLT5jaGFubmVsX21hc2sgJiBCSVQoMCkpCj4g KwkJZGVmcjYgfD0gREVGUjZfT0RQTTAyX0RJU1A7Cj4gKwo+ICsJaWYgKHJncnAtPmNoYW5uZWxf bWFzayAmIEJJVCgxKSkKPiAgCQlkZWZyNiB8PSBERUZSNl9PRFBNMTJfRElTUDsKClNvIG11Y2gg Y2xlYW5lciB3aXRoIHRoZSBjaGFubmVscyBtYXNrLCBJIGxpa2UgdGhpcy4KCj4gIAlyY2FyX2R1 X2dyb3VwX3dyaXRlKHJncnAsIERFRlI2LCBkZWZyNik7Cj4gQEAgLTgwLDEwICs4MywxMSBAQCBz dGF0aWMgdm9pZCByY2FyX2R1X2dyb3VwX3NldHVwX2RlZnI4KHN0cnVjdAo+IHJjYXJfZHVfZ3Jv dXAgKnJncnApICogT24gR2VuMyBWU1BEIHJvdXRpbmcgY2FuJ3QgYmUgY29uZmlndXJlZCwgYnV0 IERQQUQKPiByb3V0aW5nCj4gIAkJICogbmVlZHMgdG8gYmUgc2V0IGRlc3BpdGUgaGF2aW5nIGEg c2luZ2xlIG9wdGlvbiBhdmFpbGFibGUuCj4gIAkJICovCj4gLQkJdTMyIGNydGMgPSBmZnMocG9z c2libGVfY3J0Y3MpIC0gMTsKPiArCQl1bnNpZ25lZCBpbnQgcmdiX2NydGMgPSBmZnMocG9zc2li bGVfY3J0Y3MpIC0gMTsKPiArCQlzdHJ1Y3QgcmNhcl9kdV9jcnRjICpjcnRjID0gJnJjZHUtPmNy dGNzW3JnYl9jcnRjXTsKPiAKPiAtCQlpZiAoY3J0YyAvIDIgPT0gcmdycC0+aW5kZXgpCj4gLQkJ CWRlZnI4IHw9IERFRlI4X0RSR0JTX0RVKGNydGMpOwo+ICsJCWlmIChjcnRjLT5pbmRleCAvIDIg PT0gcmdycC0+aW5kZXgpCj4gKwkJCWRlZnI4IHw9IERFRlI4X0RSR0JTX0RVKGNydGMtPmluZGV4 KTsKPiAgCX0KPiAKPiAgCXJjYXJfZHVfZ3JvdXBfd3JpdGUocmdycCwgREVGUjgsIGRlZnI4KTsK PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9ncm91cC5oCj4g Yi9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2dyb3VwLmggaW5kZXggNWUzYWRjNmIz MWI1Li5kMjlhNjhlMDA2YTcKPiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vcmNhci1k dS9yY2FyX2R1X2dyb3VwLmgKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1 X2dyb3VwLmgKPiBAQCAtMjUsNiArMjUsNyBAQCBzdHJ1Y3QgcmNhcl9kdV9kZXZpY2U7Cj4gICAq IEBkZXY6IHRoZSBEVSBkZXZpY2UKPiAgICogQG1taW9fb2Zmc2V0OiByZWdpc3RlcnMgb2Zmc2V0 IGluIHRoZSBkZXZpY2UgbWVtb3J5IG1hcAo+ICAgKiBAaW5kZXg6IGdyb3VwIGluZGV4Cj4gKyAq IEBjaGFubmVsX21hc2s6IGJpdG1hc2sgb2YgcG9wdWxhdGVkIERVIGNoYW5uZWxzIGluIHRoaXMg Z3JvdXAKPiAgICogQG51bV9jcnRjczogbnVtYmVyIG9mIENSVENzIGluIHRoaXMgZ3JvdXAgKDEg b3IgMikKPiAgICogQHVzZV9jb3VudDogbnVtYmVyIG9mIHVzZXJzIG9mIHRoZSBncm91cCAocmNh cl9kdV9ncm91cF8oZ2V0fHB1dCkpCj4gICAqIEB1c2VkX2NydGNzOiBudW1iZXIgb2YgQ1JUQ3Mg Y3VycmVudGx5IGluIHVzZQo+IEBAIC0zOSw2ICs0MCw3IEBAIHN0cnVjdCByY2FyX2R1X2dyb3Vw IHsKPiAgCXVuc2lnbmVkIGludCBtbWlvX29mZnNldDsKPiAgCXVuc2lnbmVkIGludCBpbmRleDsK PiAKPiArCXVuc2lnbmVkIGludCBjaGFubmVsX21hc2s7CgpEZXBlbmRpbmcgb24gaG93IHlvdSBs aWtlIG15IHN1Z2dlc3Rpb24gaW4gcGF0Y2ggMDUvMTcsIHRoaXMgbWlnaHQgYmUgYmV0dGVyIApj YWxsZWQgY2hhbm5lbHNfbWFzay4KCj4gIAl1bnNpZ25lZCBpbnQgbnVtX2NydGNzOwo+ICAJdW5z aWduZWQgaW50IHVzZV9jb3VudDsKPiAgCXVuc2lnbmVkIGludCB1c2VkX2NydGNzOwo+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2ttcy5jCj4gYi9kcml2ZXJz L2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X2ttcy5jIGluZGV4IDE5YTQ0NWZiYzg3OS4uNDVmYjU1 NGZkM2M3Cj4gMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9r bXMuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfa21zLmMKPiBAQCAt NTk3LDcgKzU5NywxMCBAQCBpbnQgcmNhcl9kdV9tb2Rlc2V0X2luaXQoc3RydWN0IHJjYXJfZHVf ZGV2aWNlICpyY2R1KQo+ICAJCXJncnAtPmRldiA9IHJjZHU7Cj4gIAkJcmdycC0+bW1pb19vZmZz ZXQgPSBtbWlvX29mZnNldHNbaV07Cj4gIAkJcmdycC0+aW5kZXggPSBpOwo+IC0JCXJncnAtPm51 bV9jcnRjcyA9IG1pbihyY2R1LT5udW1fY3J0Y3MgLSAyICogaSwgMlUpOwo+ICsJCS8qIEV4dHJh Y3QgdGhlIGNoYW5uZWwgbWFzayBmb3IgdGhpcyBncm91cCBvbmx5ICovCgpzL29ubHkvb25seS4v Cgo+ICsJCXJncnAtPmNoYW5uZWxfbWFzayA9IChyY2R1LT5pbmZvLT5jaGFubmVsX21hc2sgPj4g KDIgKiBpKSkKPiArCQkJCSAgICYgR0VOTUFTSygxLCAwKTsKPiArCQlyZ3JwLT5udW1fY3J0Y3Mg PSBod2VpZ2h0OChyZ3JwLT5jaGFubmVsX21hc2spOwoKWW91IGNvdWxkIG9wdGltaXplIHRoaXMg YnkgY29tcHV0aW5nIGl0IGFzCgoJcmdycC0+bnVtX2NydGNzID0gKHJncnAtPmNoYW5uZWxfbWFz ayA+PiAxKQoJCQl8IChyZ3JwLT5jaGFubmVsX21hc2sgJiAxKTsKCmFzIHlvdSBrbm93IHRoYXQg b25seSB0d28gYml0cyBhdCBtb3N0IGNhbiBiZSBzZXQuIFVwIHRvIHlvdS4KCj4gIAkJLyoKPiAg CQkgKiBJZiB3ZSBoYXZlIG1vcmUgdGhhbiBvbmUgQ1JUQ3MgaW4gdGhpcyBncm91cCBwcmUtYXNz b2NpYXRlCgpXaXRoIHRob3NlIHNtYWxsIGlzc3VlcyBmaXhlZCwKClJldmlld2VkLWJ5OiBMYXVy ZW50IFBpbmNoYXJ0IDxsYXVyZW50LnBpbmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+CgotLSAKUmVn YXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vZHJpLWRldmVsCg==