From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH] drm/omap: fix primary-plane's possible_crtcs Date: Thu, 1 Dec 2016 14:59:28 +0200 Message-ID: <7106a82d-acae-2aca-7e90-2cf9205c56e1@ti.com> References: <1480502331-6635-1-git-send-email-tomi.valkeinen@ti.com> <70503474.pOHboTESKv@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1383720183==" Return-path: Received: from fllnx209.ext.ti.com (fllnx209.ext.ti.com [198.47.19.16]) by gabe.freedesktop.org (Postfix) with ESMTPS id 23EF86E06A for ; Thu, 1 Dec 2016 12:59:35 +0000 (UTC) In-Reply-To: <70503474.pOHboTESKv@avalon> 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: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1383720183== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AfITQhXS4TCiVLlrJ3DO8V9OXBklTmKeo" --AfITQhXS4TCiVLlrJ3DO8V9OXBklTmKeo Content-Type: multipart/mixed; boundary="qwbMwCK5wRjj76vBwbRLXOogkTtREbs5M"; protected-headers="v1" From: Tomi Valkeinen To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Message-ID: <7106a82d-acae-2aca-7e90-2cf9205c56e1@ti.com> Subject: Re: [PATCH] drm/omap: fix primary-plane's possible_crtcs References: <1480502331-6635-1-git-send-email-tomi.valkeinen@ti.com> <70503474.pOHboTESKv@avalon> In-Reply-To: <70503474.pOHboTESKv@avalon> --qwbMwCK5wRjj76vBwbRLXOogkTtREbs5M Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 30/11/16 16:36, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > On Wednesday 30 Nov 2016 12:38:51 Tomi Valkeinen wrote: >> We set the possible_crtc for all planes to "(1 << priv->num_crtcs) - 1= ", >> which is fine as the HW planes can be used fro all crtcs. However, whe= n >> we're doing that, we are still incrementing 'num_crtcs', and we'll end= >> up with bad possible_crtcs, preventing the use of the primary planes. >> >> We should have all crtcs in 'possible_crtc', but apparently it's not a= s >> easy to set as you would think. We create crtcs rather dynamically, an= d >> when creating the primary planes, we don't know how many crtcs we're >> going to have. This is mostly a problem with the way omapdrm creates >> crtcs and planes, and how it connects those to display outputs. >> >> So, this patch fixes the problem the easy way, and sets the >> possible_crtcs for primary planes only to the crtc in question, which = in >> practice should cover all normal use cases. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/omap_plane.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c >> b/drivers/gpu/drm/omapdrm/omap_plane.c index 9c43cb481e62..fc1822870b2= 6 >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_plane.c >> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c >> @@ -361,6 +361,7 @@ struct drm_plane *omap_plane_init(struct drm_devic= e >> *dev, struct omap_drm_private *priv =3D dev->dev_private; >> struct drm_plane *plane; >> struct omap_plane *omap_plane; >> + unsigned long possible_crtcs; >> int ret; >> >> DBG("%s: type=3D%d", plane_names[id], type); >> @@ -381,7 +382,12 @@ struct drm_plane *omap_plane_init(struct drm_devi= ce >> *dev, omap_plane->error_irq.irq =3D omap_plane_error_irq; >> omap_irq_register(dev, &omap_plane->error_irq); >> >> - ret =3D drm_universal_plane_init(dev, plane, (1 << priv->num_crtcs) = - 1, >> + if (type =3D=3D DRM_PLANE_TYPE_PRIMARY) >> + possible_crtcs =3D 1 << id; >> + else >> + possible_crtcs =3D (1 << priv->num_crtcs) - 1; >=20 > The omap_modeset_init() function computes the number of CRTCs before cr= eating=20 > any plane with >=20 > num_crtcs =3D min3(num_crtc, num_mgrs, num_ovls); >=20 > and the rest of the function then creates CRTCs, as far as I understand= always=20 > up to num_crtcs. Can't we just use that value to compute possible_crtcs= as (1=20 > << num_crtcs ) - 1 ? Yes, I think you're correct. It's not exactly obvious from the code =3D).= I'll change the patch to use the calculated num-crtcs. Tomi --qwbMwCK5wRjj76vBwbRLXOogkTtREbs5M-- --AfITQhXS4TCiVLlrJ3DO8V9OXBklTmKeo Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYQB6wAAoJEPo9qoy8lh71RogP/0aFoGd6zako8DhxPH99IapA HAzbXySOso5O93mHBLbOtnZ917juM1p19wPpRvjlRBhf+JNAuOPpdJPZL1pw8C52 zfHCBPxR55z675+ju03TmcNXPD24K0kOBAdeNNdn9K/+SSv0vGXDC0pwfMbhNVAX o7ILNhKLIBtRwPoBTD8C8RcoATyX51NiUZD9duWQt6wFlZE5AWnxgbWSiW9PQ9nB SWP66Nf6KSHyyvTJHBB0xYhaHg/IOCJmP1m3FPnQmxMLM1dn0VP7MRIL2dg86V+8 ZlbJpSvW/fqX8+ir47KFBAapNusd+9X/DRJ6LnQFtOjJs1thTWV1DYm/dL/OgQHd FTL/CEa0NwWtECRvuhHgRG8/9SSyvNRnmtZcj0FiVVWcbdWvErAZM8gruI2x0bH8 B4UBUhtY5JUx8rKRtAIDcBAwM3gxuDJlmQ1KbuSd2SCm7Q2fjy2/hUVc5UvUvLYH 4UBX5FgoDLjKlnGeo8X3p83g2ACGDISepuxEl/qxLAa4ycrZet2+vLVfiypBLuPp tA3GTeyvpBDHn2SuOszU37Nk0ZfRlFKNmh/OwRdG5bMPjMjAa5sx5nzULs1ZT99l AeiZTkM/gMUSYUn8pmtVKdXZMCqNqJARbUUCRWkBjer4bYrxdHdbKuI0kXLRZC+i ONAxPsM9Vg8yN+l712dz =4OOH -----END PGP SIGNATURE----- --AfITQhXS4TCiVLlrJ3DO8V9OXBklTmKeo-- --===============1383720183== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1383720183==--