From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay4-d.mail.gandi.net ([217.70.183.196]:39987 "EHLO relay4-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726622AbeIKUqv (ORCPT ); Tue, 11 Sep 2018 16:46:51 -0400 Date: Tue, 11 Sep 2018 17:46:53 +0200 From: jacopo mondi To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH 08/16] drm: rcar-du: Enable configurable DPAD0 routing on Gen3 Message-ID: <20180911154653.GW28160@w540> References: <20180904121027.24031-1-laurent.pinchart+renesas@ideasonboard.com> <20180904121027.24031-9-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="aJDJANv8BPX70wwH" Content-Disposition: inline In-Reply-To: <20180904121027.24031-9-laurent.pinchart+renesas@ideasonboard.com> Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: --aJDJANv8BPX70wwH Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Hi Laurent, On Tue, Sep 04, 2018 at 03:10:19PM +0300, Laurent Pinchart wrote: > All Gen3 SoCs supported so far have a fixed association between DPAD0 > and DU channels, which led to hardcoding that association when writing > the corresponding hardware register. The D3 and E3 will break that > mechanism as DPAD0 can be dynamically connected to either DU0 or DU1. > > Make DPAD0 routing dynamic on Gen3. To ensure a valid hardware > configuration when the DU starts without the RGB output enabled, DPAD0 > is associated at initialization time to the first DU channel that it can > be connected to. This makes no change on Gen2 as all Gen2 SoCs can > connected DPAD0 to DU0, which is the current implicit default value. > > As the DPAD0 source is always 0 when a single source is possible on > Gen2, we can also simplify the Gen2 code in the same function to remove > a conditional check. > Does this patch only prepares for routing to be mad dynamic or it is already supported? I am missing where those dynamic association happens, as I see the possible dpad0 source being changed in 'rcar_du_crtc_route_output()' which is in turn called by the mode set operation 'rcar_du_crtc_route_output()'. But at the same time, I only see the routing register DEFR8 being written by rcar_du_group_setup_defr8() called by the group route setup operation rcar_du_group_set_routing() called at crtc setup time only. Would the mode set operation on the encoder being supporsed to go to the group's set_routing operation? Or does DEFR8 configuration happens with a different call path? Thanks j > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_group.c | 17 ++++++----------- > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 12 ++++++++++++ > 2 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c > index 4c62841eff2f..f38703e7a10d 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -56,8 +56,6 @@ static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp) > static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) > { > struct rcar_du_device *rcdu = rgrp->dev; > - unsigned int possible_crtcs = > - rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs; > u32 defr8 = DEFR8_CODE; > > if (rcdu->info->gen < 3) { > @@ -69,21 +67,18 @@ static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) > * DU instances that support it. > */ > if (rgrp->index == 0) { > - if (possible_crtcs > 1) > - defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > if (rgrp->dev->vspd1_sink == 2) > defr8 |= DEFR8_VSCS; > } > } else { > /* > - * On Gen3 VSPD routing can't be configured, but DPAD routing > - * needs to be set despite having a single option available. > + * On Gen3 VSPD routing can't be configured, and DPAD routing > + * is set in the group corresponding to the DPAD output (no Gen3 > + * SoC has multiple DPAD sources belonging to separate groups). > */ > - unsigned int rgb_crtc = ffs(possible_crtcs) - 1; > - struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc]; > - > - if (crtc->index / 2 == rgrp->index) > - defr8 |= DEFR8_DRGBS_DU(crtc->index); > + if (rgrp->index == rcdu->dpad0_source / 2) > + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > } > > rcar_du_group_write(rgrp, DEFR8, defr8); > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > index ed7fa3204892..bd01197700c5 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -501,6 +501,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > struct drm_device *dev = rcdu->ddev; > struct drm_encoder *encoder; > struct drm_fbdev_cma *fbdev; > + unsigned int dpad0_sources; > unsigned int num_encoders; > unsigned int num_groups; > unsigned int swindex; > @@ -613,6 +614,17 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > encoder->possible_clones = (1 << num_encoders) - 1; > } > > + /* > + * Initialize the default DPAD0 source to the index of the first DU > + * channel that can be connected to DPAD0. The exact value doesn't > + * matter as it should be overwritten by mode setting for the RGB > + * output, but it is nonetheless required to ensure a valid initial > + * hardware configuration on Gen3 where DU0 can't always be connected to > + * DPAD0. > + */ > + dpad0_sources = rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs; > + rcdu->dpad0_source = ffs(dpad0_sources) - 1; > + > drm_mode_config_reset(dev); > > drm_kms_helper_poll_init(dev); > -- > Regards, > > Laurent Pinchart > --aJDJANv8BPX70wwH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJbl+NtAAoJEHI0Bo8WoVY8kMAQAMO4Ludcegrj9mWKh4bEh6A5 mnja+nJNk5XYxoYG+9lNeU0IT/PCt+Ufy4pmca8PB9jKSiydB25Sipueh89rE0Lw hLOzltYZI8EhBtJbEQ2hDNaDCIh6WfRq9hg7Id1TrVe5Ew+mG+yEqb6Mzn8Dq0Hh RtHxbtXqOqDlivurw9ilDi/PQ4DeTfAxEdfbyy0uYDwpM6FLu8sNdPNRD6CugmNZ 5zZ2IyHPcBK3Gz4e08B+u3E7cnpvYM12jdWIEQp9wkDYEnA1fb7ypQAXHEVQ8FOe KEcHu3rwD/R5ALMr/v0ehJISy87cfeZNrVhE3OP9SYspXbMuDZSHw0ue8r5Jwk0n qwV6XR0YNa6KQeY5dINBeoXeIgUbACnunKDBEM9ulm/uJG5MJ0mmn1wUkbUV1NHa xz/VDRh+gKKhvj4r4JTBToedHEAa/VA8R5OsoTEvgpVcV4P978iDPC8bNFL4DDIh zRemh/BM+LuzKVq4D3sNi6C6r8ob9gQMuxGDZIICgG6/C264f4UPgyiPBMKM+qHL 8iXXRyRTFV6tkIdiJ0zIEXyj6/X0PqtJ5GQ8pUxFkwgU8ztbBU5h7xH6RFcmvLgQ LaxO7pkpapmQRzOTbag5aeHdEqLD3AfQFpZ0d0FmyTJKU7SDTaB/Wfpahj8+4LE0 6pSpwg4H5loLXnSR0oDw =Wpxd -----END PGP SIGNATURE----- --aJDJANv8BPX70wwH-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: jacopo mondi Subject: Re: [PATCH 08/16] drm: rcar-du: Enable configurable DPAD0 routing on Gen3 Date: Tue, 11 Sep 2018 17:46:53 +0200 Message-ID: <20180911154653.GW28160@w540> References: <20180904121027.24031-1-laurent.pinchart+renesas@ideasonboard.com> <20180904121027.24031-9-laurent.pinchart+renesas@ideasonboard.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1544839324==" Return-path: Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2C51588E40 for ; Tue, 11 Sep 2018 15:46:57 +0000 (UTC) In-Reply-To: <20180904121027.24031-9-laurent.pinchart+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: Laurent Pinchart Cc: linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1544839324== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="aJDJANv8BPX70wwH" Content-Disposition: inline --aJDJANv8BPX70wwH Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Hi Laurent, On Tue, Sep 04, 2018 at 03:10:19PM +0300, Laurent Pinchart wrote: > All Gen3 SoCs supported so far have a fixed association between DPAD0 > and DU channels, which led to hardcoding that association when writing > the corresponding hardware register. The D3 and E3 will break that > mechanism as DPAD0 can be dynamically connected to either DU0 or DU1. > > Make DPAD0 routing dynamic on Gen3. To ensure a valid hardware > configuration when the DU starts without the RGB output enabled, DPAD0 > is associated at initialization time to the first DU channel that it can > be connected to. This makes no change on Gen2 as all Gen2 SoCs can > connected DPAD0 to DU0, which is the current implicit default value. > > As the DPAD0 source is always 0 when a single source is possible on > Gen2, we can also simplify the Gen2 code in the same function to remove > a conditional check. > Does this patch only prepares for routing to be mad dynamic or it is already supported? I am missing where those dynamic association happens, as I see the possible dpad0 source being changed in 'rcar_du_crtc_route_output()' which is in turn called by the mode set operation 'rcar_du_crtc_route_output()'. But at the same time, I only see the routing register DEFR8 being written by rcar_du_group_setup_defr8() called by the group route setup operation rcar_du_group_set_routing() called at crtc setup time only. Would the mode set operation on the encoder being supporsed to go to the group's set_routing operation? Or does DEFR8 configuration happens with a different call path? Thanks j > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_group.c | 17 ++++++----------- > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 12 ++++++++++++ > 2 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c > index 4c62841eff2f..f38703e7a10d 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c > @@ -56,8 +56,6 @@ static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp) > static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) > { > struct rcar_du_device *rcdu = rgrp->dev; > - unsigned int possible_crtcs = > - rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs; > u32 defr8 = DEFR8_CODE; > > if (rcdu->info->gen < 3) { > @@ -69,21 +67,18 @@ static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp) > * DU instances that support it. > */ > if (rgrp->index == 0) { > - if (possible_crtcs > 1) > - defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > if (rgrp->dev->vspd1_sink == 2) > defr8 |= DEFR8_VSCS; > } > } else { > /* > - * On Gen3 VSPD routing can't be configured, but DPAD routing > - * needs to be set despite having a single option available. > + * On Gen3 VSPD routing can't be configured, and DPAD routing > + * is set in the group corresponding to the DPAD output (no Gen3 > + * SoC has multiple DPAD sources belonging to separate groups). > */ > - unsigned int rgb_crtc = ffs(possible_crtcs) - 1; > - struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc]; > - > - if (crtc->index / 2 == rgrp->index) > - defr8 |= DEFR8_DRGBS_DU(crtc->index); > + if (rgrp->index == rcdu->dpad0_source / 2) > + defr8 |= DEFR8_DRGBS_DU(rcdu->dpad0_source); > } > > rcar_du_group_write(rgrp, DEFR8, defr8); > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > index ed7fa3204892..bd01197700c5 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c > @@ -501,6 +501,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > struct drm_device *dev = rcdu->ddev; > struct drm_encoder *encoder; > struct drm_fbdev_cma *fbdev; > + unsigned int dpad0_sources; > unsigned int num_encoders; > unsigned int num_groups; > unsigned int swindex; > @@ -613,6 +614,17 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > encoder->possible_clones = (1 << num_encoders) - 1; > } > > + /* > + * Initialize the default DPAD0 source to the index of the first DU > + * channel that can be connected to DPAD0. The exact value doesn't > + * matter as it should be overwritten by mode setting for the RGB > + * output, but it is nonetheless required to ensure a valid initial > + * hardware configuration on Gen3 where DU0 can't always be connected to > + * DPAD0. > + */ > + dpad0_sources = rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs; > + rcdu->dpad0_source = ffs(dpad0_sources) - 1; > + > drm_mode_config_reset(dev); > > drm_kms_helper_poll_init(dev); > -- > Regards, > > Laurent Pinchart > --aJDJANv8BPX70wwH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJbl+NtAAoJEHI0Bo8WoVY8kMAQAMO4Ludcegrj9mWKh4bEh6A5 mnja+nJNk5XYxoYG+9lNeU0IT/PCt+Ufy4pmca8PB9jKSiydB25Sipueh89rE0Lw hLOzltYZI8EhBtJbEQ2hDNaDCIh6WfRq9hg7Id1TrVe5Ew+mG+yEqb6Mzn8Dq0Hh RtHxbtXqOqDlivurw9ilDi/PQ4DeTfAxEdfbyy0uYDwpM6FLu8sNdPNRD6CugmNZ 5zZ2IyHPcBK3Gz4e08B+u3E7cnpvYM12jdWIEQp9wkDYEnA1fb7ypQAXHEVQ8FOe KEcHu3rwD/R5ALMr/v0ehJISy87cfeZNrVhE3OP9SYspXbMuDZSHw0ue8r5Jwk0n qwV6XR0YNa6KQeY5dINBeoXeIgUbACnunKDBEM9ulm/uJG5MJ0mmn1wUkbUV1NHa xz/VDRh+gKKhvj4r4JTBToedHEAa/VA8R5OsoTEvgpVcV4P978iDPC8bNFL4DDIh zRemh/BM+LuzKVq4D3sNi6C6r8ob9gQMuxGDZIICgG6/C264f4UPgyiPBMKM+qHL 8iXXRyRTFV6tkIdiJ0zIEXyj6/X0PqtJ5GQ8pUxFkwgU8ztbBU5h7xH6RFcmvLgQ LaxO7pkpapmQRzOTbag5aeHdEqLD3AfQFpZ0d0FmyTJKU7SDTaB/Wfpahj8+4LE0 6pSpwg4H5loLXnSR0oDw =Wpxd -----END PGP SIGNATURE----- --aJDJANv8BPX70wwH-- --===============1544839324== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1544839324==--