From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 4/9] drm/omap: make modesetting synchronous Date: Mon, 8 Sep 2014 15:53:18 +0300 Message-ID: <540DA6BE.3020406@ti.com> References: <1409745310-19092-1-git-send-email-tomi.valkeinen@ti.com> <1409745310-19092-4-git-send-email-tomi.valkeinen@ti.com> <20140903142525.GS15520@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1659556715==" Return-path: Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by gabe.freedesktop.org (Postfix) with ESMTP id 2DE426E2A1 for ; Mon, 8 Sep 2014 05:53:23 -0700 (PDT) In-Reply-To: <20140903142525.GS15520@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1659556715== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Pxqx0V2lVuc964OeR6eLHFOrcewLftNcx" --Pxqx0V2lVuc964OeR6eLHFOrcewLftNcx Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 03/09/14 17:25, Daniel Vetter wrote: > On Wed, Sep 03, 2014 at 02:55:05PM +0300, Tomi Valkeinen wrote: >> Currently modesetting is not done synchronously, but it queues work th= at >> is done later. In theory this is fine, but the driver doesn't handle i= t >> at properly. This means that if an application first does a full >> modeset, then immediately afterwards starts page flipping, the page >> flipping will not work properly as there's modeset work still in the >> queue. >> >> The result with my test application was that a backbuffer was shown on= >> the screen. >> >> Fixing this properly would be rather big undertaking. Thus this patch >> fixes the issue by making the modesetting synchronous, by waiting for >> the queued work to be done at the end of omap_crtc->commit(). >> >> The ugly part here is that the background work takes crtc->mutex, and >> the modesetting also holds that lock, which means that the background >> work never gets done. To get around this, we unclock, wait, and lock >> again. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/omap_crtc.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/oma= pdrm/omap_crtc.c >> index 193979f97bdb..3261fbf94957 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c >> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c >> @@ -277,8 +277,13 @@ static void omap_crtc_prepare(struct drm_crtc *cr= tc) >> static void omap_crtc_commit(struct drm_crtc *crtc) >> { >> struct omap_crtc *omap_crtc =3D to_omap_crtc(crtc); >> + struct drm_device *dev =3D crtc->dev; >> DBG("%s", omap_crtc->name); >> omap_crtc_dpms(crtc, DRM_MODE_DPMS_ON); >> + >> + drm_modeset_unlock_all(dev); >=20 > This will run afoul of the upcoming locking rework in the atomic work. = And > I'm fairly sure that the crtc helpers will fall over badly if someone > submits a concurrent setCrtc while you've dropped the locks here. >=20 > Can't you instead just drop the locking from the worker? As long as you= > synchronize like here at the right places it can't race. I expect that = you > might want to synchronize in the crtc_prepare hook, too. But beyond tha= t > it should work. >=20 > As-is nacked because future headaches for me ;-) Yes, it's ugly. But doing it with either queuing or caching would be a major change. I was just trying to do smallish fixes to the driver for no= w. How about only unlocking/locking the crtc->mutex as Rob suggested? I think it should work, but I didn't try it yet. Would that be as bad for the locking rework? Tomi --Pxqx0V2lVuc964OeR6eLHFOrcewLftNcx Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUDaa+AAoJEPo9qoy8lh71HiAP/RlUEXbYd0g3TgSotIKr+jTz uKNdmQqBBmXeSOUTj6JA0zEqvP5ExSK1sA7hiZxLWjdn7pCa96zk4NnOkOGZWMzO YCJvYCXrFfgdhlhvU5iNSHaaJ4UKhXucgTpxteaucD9WMR2P+wGDF4vDoBIwuSAg tB+u+jFs55c95imr1TbVCp3eUxG3+4lLEaKPEPgI9Jn23wH8lspzFwXz3odjkfJ1 uKN4HhuouZLsLHro6QO6RgOYHPQ7VYJLWwfeXd7gB4paTQLSIszUvasi5MMOTrn1 KblsFUJenGJQ+X0BENDbB2eHiHVCkHMPylyBWcy5cx2clIpRIGxsgGZ56YDitl/r NPts+V2afgC7o83NjrJnERg1MpRR6Jc5goA77B0CtJ/YiVFqUCBYXz5KilHgxaV3 p/gk82wJcfxleXooRqVq2jmYYdpuMYt3gfKl4vCgHU4P6lG5fCjRCzxTNIdbFBLX aUr6Ihmsi85YOq23GQx/BzMf/b3Av9tmhNeWIyzjoJ6wedD4d0eKnYfYEpoaSS5Q ++m0ekAiGa7HbBuCKexztqId7bsTKBe7PljoSQYGFlV5Hdybz8y+dD2jhdQ047Vq Xm2pFy1m0QB4I7ucilN1yqgb/Dv1ydXN+829gHDt04j2MQ4GHyz8s/kFQvHk+uQQ p1rxF1nw1hjrU0VR4Ymj =wh++ -----END PGP SIGNATURE----- --Pxqx0V2lVuc964OeR6eLHFOrcewLftNcx-- --===============1659556715== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1659556715==--