From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [Intel-gfx] [PATCH 07/15] drm/omap: Use per-plane rotation property Date: Fri, 23 Sep 2016 14:33:37 +0300 Message-ID: <98bb2235-b6da-7f07-e878-a103ba066b72@ti.com> References: <1469194996-1899-1-git-send-email-ville.syrjala@linux.intel.com> <1469194996-1899-8-git-send-email-ville.syrjala@linux.intel.com> <2e3e4c0f-a1f6-e192-3993-cce12304feaa@gmail.com> <20160811133332.GL4329@intel.com> <20160812160404.GD4329@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0501725848==" Return-path: Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [198.47.19.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id 32FB96E3F1 for ; Fri, 23 Sep 2016 11:33:42 +0000 (UTC) In-Reply-To: <20160812160404.GD4329@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Tomi Valkeinen Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0501725848== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KSJKDBFNcuumCAeTPawqk3Vi1D4KQQbqq" --KSJKDBFNcuumCAeTPawqk3Vi1D4KQQbqq Content-Type: multipart/mixed; boundary="E5xNDmhkINvCu2rxWnwRVIppeENUgD4T9"; protected-headers="v1" From: Tomi Valkeinen To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Tomi Valkeinen Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Message-ID: <98bb2235-b6da-7f07-e878-a103ba066b72@ti.com> Subject: Re: [Intel-gfx] [PATCH 07/15] drm/omap: Use per-plane rotation property References: <1469194996-1899-1-git-send-email-ville.syrjala@linux.intel.com> <1469194996-1899-8-git-send-email-ville.syrjala@linux.intel.com> <2e3e4c0f-a1f6-e192-3993-cce12304feaa@gmail.com> <20160811133332.GL4329@intel.com> <20160812160404.GD4329@intel.com> In-Reply-To: <20160812160404.GD4329@intel.com> --E5xNDmhkINvCu2rxWnwRVIppeENUgD4T9 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/08/16 19:04, Ville Syrj=C3=A4l=C3=A4 wrote: > On Thu, Aug 11, 2016 at 04:33:32PM +0300, Ville Syrj=C3=A4l=C3=A4 wrote= : >> On Thu, Aug 11, 2016 at 02:32:44PM +0300, Tomi Valkeinen wrote: >>> Hi, >>> >>> On 22/07/16 16:43, ville.syrjala@linux.intel.com wrote: >>>> From: Ville Syrj=C3=A4l=C3=A4 >>>> >>>> The global mode_config.rotation_property is going away, switch over = to >>>> per-plane rotation_property. >>>> >>>> Not sure I got the annoying crtc rotation_property handling right. >>>> Might work, or migth not. >>> >>> I think something is funny with this patch or the series. I fetched y= our >>> branch, and with your series, it looks like the primary planes lose a= ll >>> their props. modetest says: >>> >>> could not get plane 26 properties: Invalid argument >>> could not get plane 30 properties: Invalid argument >> >> Hmm. Weird. Is it really the get props ioctl that fails? >> >> The first EINVAL I can spot there is >> if (!obj->properties) { >> ret =3D -EINVAL; >> goto out_unref; >> } >> which definitely makes no sense since this is assigned >> as plane->base.properties =3D &plane->properties. So can't be that unl= ess >> we manage to clear the pointer somehow after the init. >> >> The only other direct EINVAL I see there is if >> drm_object_property_get_value(obj->properties->properties[i]) >> fails to find the passed prop in the properties array. Which clearly >> can't happen since we got it from the array in the first place. Also, >> clearly that code is rather inefficient, perhaps someone should rewrit= e >> it a bit. >> >> Can't quite see how this could fail for the plane in other ways. But I= >> might be blind. >=20 > I tried to think on this a bit more, and the only think I came up with = was > that we end up doing the drm_plane_create_rotation_property() twice for= the > primary planes. I tried that on i915 but it'd didn't result in anything= bad > AFAICS. Would leak a bit, but so what :P >=20 > Dunno, I guess you could try something like: >=20 > --- a/drivers/gpu/drm/omapdrm/omap_plane.c > +++ b/drivers/gpu/drm/omapdrm/omap_plane.c > @@ -211,11 +211,12 @@ void omap_plane_install_properties(struct drm_pla= ne *plane, > struct omap_drm_private *priv =3D dev->dev_private; > =20 > if (priv->has_dmm) { > - drm_plane_create_rotation_property(plane, > - BIT(DRM_ROTATE_0), > - BIT(DRM_ROTATE_0) | = BIT(DRM_ROTATE_90) | > - BIT(DRM_ROTATE_180) = | BIT(DRM_ROTATE_270) | > - BIT(DRM_REFLECT_X) |= BIT(DRM_REFLECT_Y)); > + if (!plane->rotation_property) > + drm_plane_create_rotation_property(plane, > + BIT(DRM_ROTA= TE_0), > + BIT(DRM_ROTA= TE_0) | BIT(DRM_ROTATE_90) | > + BIT(DRM_ROTA= TE_180) | BIT(DRM_ROTATE_270) | > + BIT(DRM_REFL= ECT_X) | BIT(DRM_REFLECT_Y)); This fixes the problem... Obviously something gets confused if the property is created twice. Perhaps the first one gets stored somewhere, and gets used somehow, even if the latter property is the "real" one, attached to the plane? Just a guess... Tomi --E5xNDmhkINvCu2rxWnwRVIppeENUgD4T9-- --KSJKDBFNcuumCAeTPawqk3Vi1D4KQQbqq 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 iQIcBAEBCAAGBQJX5RMRAAoJEPo9qoy8lh71VX4P/R6FML9rDzki9eLdPdVpXJLr gtn6KvR+w7/32mpbdzC2rPvlsQILXyeWIUOJMegEHXm2Pmjjm+g+BaIhLAusS9T2 WitwW6DtvX70O9A+fMIPY7cEMf6iSsZGjojhxpKybY7ldQNHzjGy6fL5BDImLzkb XEBLb+INnGDpkEAY83kz2DU+HjewwIAos8Xu68ipvh7iAD3xjmB4DxryMu/Np0si 8+yFEN6SISrtuO+1y59JtxEt8JOQQfy5XuaJI/C4WtkAz5037lSyL1/ay9AzWy+S U5Mo6+T/3aY+/hAckC+UOz15V6DC77fvybyoAKC0cMKPnU67q1FVR2n+q1cD1E78 DDHRRnGF4AsggkB3lLCKSeW199Glqw7ZxQij7OeAjFKTIjVTRtSmpaqjAAs+kjog auztfseZ2pMTZ3gnCxWCyQ2fUA2fc0yxF+h3X7A483HJfuPWHdv/cXQCnl+QZ0HU KbHKLM8+QYIUkaNE6YW6065AC9Xyd3U27doDh908yRGe8mPhS+E2W2r12LvqNkPG THRXvpDRKBQBi9hk3fr5kHszwSVL0otip8y3E3IeASRNbytK1N5+Z51QJp9Wzgmy 2mqZZZ1psT+03YSaWPAD3iqZJLXmHTitDCtn/XgpHp3xpc9GlI7RmcvuXn3bcA+s JL42cK6elT1hS4EuJQeM =uHaG -----END PGP SIGNATURE----- --KSJKDBFNcuumCAeTPawqk3Vi1D4KQQbqq-- --===============0501725848== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0501725848==--