From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 2/5] drm/omap: check that plane is inside crtc Date: Tue, 23 Jun 2015 11:01:24 +0300 Message-ID: <55891254.6060404@ti.com> References: <1434622239-15629-1-git-send-email-tomi.valkeinen@ti.com> <1434622239-15629-3-git-send-email-tomi.valkeinen@ti.com> <3022545.7m5qkGr2vj@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1357154862==" Return-path: Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) by gabe.freedesktop.org (Postfix) with ESMTP id CA18189A83 for ; Tue, 23 Jun 2015 01:01:30 -0700 (PDT) In-Reply-To: <3022545.7m5qkGr2vj@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 --===============1357154862== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bcq1X4C83WxUb7AQG2cFBxsS6bu8DO60Q" --bcq1X4C83WxUb7AQG2cFBxsS6bu8DO60Q Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 18/06/15 17:45, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > On Thursday 18 June 2015 13:10:36 Tomi Valkeinen wrote: >> DRM allows planes to be partially off-screen, but DSS hardware does no= t. >> This patch adds the necessary check to reject plane configs if the pla= ne >> is not fully inside the crtc. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/omap_plane.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c >> b/drivers/gpu/drm/omapdrm/omap_plane.c index cfa8276c4deb..53594a3b4e9= 8 >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_plane.c >> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c >> @@ -17,6 +17,7 @@ >> * this program. If not, see . >> */ >> >> +#include >> #include >> #include >> >> @@ -153,9 +154,34 @@ static void omap_plane_atomic_disable(struct drm_= plane >> *plane, dispc_ovl_enable(omap_plane->id, false); >> } >> >> +static int omap_plane_atomic_check(struct drm_plane *plane, >> + struct drm_plane_state *state) >> +{ >> + struct drm_crtc_state *crtc_state; >> + >> + if (!state->crtc) >> + return 0; >> + >> + crtc_state =3D drm_atomic_get_crtc_state(state->state, state->crtc);= >> + if (!crtc_state) >> + return 0; >=20 > drm_atomic_get_crtc_state() returns an ERR_PTR on error. You should the= n=20 > propagate the error to the caller: >=20 > crtc_state =3D drm_atomic_get_crtc_state(state->state, state->crtc); > if (IS_ERR(crtc_state)) > return PTR_ERR(crtc_state); Thanks, I missed that. I've made the change. >> + if (state->crtc_x < 0 || state->crtc_y < 0) >> + return -EINVAL; >> + >> + if (state->crtc_x + state->crtc_w > crtc_state->adjusted_mode.hdispl= ay) >> + return -EINVAL; >> + >> + if (state->crtc_y + state->crtc_h > crtc_state->adjusted_mode.vdispl= ay) >> + return -EINVAL; >=20 > I wonder whether we couldn't clip the plane in software instead of fail= ing.=20 > This patch is fine though (except for the problem above), clipping can = be=20 > implemented in a separate patch. I had the same thought, but I didn't want to go that way yet. I have a feeling that it could have corner cases that need to be taken care of, and I just wanted to fix the current behavior. Tomi --bcq1X4C83WxUb7AQG2cFBxsS6bu8DO60Q 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 iQIcBAEBCAAGBQJViRJUAAoJEPo9qoy8lh71GTIQAKIEtHsTg2yrQRqDMQ9uWQ4F G9Qnp3QzjfVkHOZClvjWWZJHWRlQ/RcuDGyH8emn9NsPictPoqalWLelqm9LwODY PuxDoRr9FFso0b64tGIGhteVY1AXqKnEyXvu76V5M+wy5UsKVSBA9kdLrzbvNrrl 3PmQXyh4i5R1g5WVhWZfZKx7JpjBUu6G8pk2bB57E4T22QaQhqhs9clDEcrsqTGW FIZNRg9g1Gh7VdJnPUxokiyIGyLzaCGjxQdu0T8NpgzDIDmuxalQtbRKDJGNNk4K pMSe+O6NXvHd4d55KXk8vPRO6hi9IwK6KEvwf6OPX0yPoGna+TFFzr10xc/3cjz0 ONJ6NescpIq9IJTBsu7pSg+sa+BBRAXrpYrbynL2qjffH5QzvOGNqNdXE47aWCy2 bzAhZ33AzjKcjWyAbbXUX66kpJRR+rjDcANNsBKC8LMT/U2f01O+Zpcdo1T4Pfcd nn7CIRd9zWb6FsMk/IRufZNKxEHACIedTIrt35qrNQuybhP0k1+KqpRnj7VJWyWP N4BJVJSxg66U9RVWD0c0N4IDkK7Y4ZUyg6NO19JMKKCKH+wImWqKTtlonIX6TwTI 994tOuv3fCbA6VwtM5v2no1kLO23mRI2OtgeukHcFPqooj8wNpNL7ZDXAbEY2xKM RmDcx61sWDsQmWZjIebq =9YC2 -----END PGP SIGNATURE----- --bcq1X4C83WxUb7AQG2cFBxsS6bu8DO60Q-- --===============1357154862== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1357154862==--