From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH v2 1/2] drm/vc4: Fix negative X/Y positioning on SAND planes Date: Thu, 06 Dec 2018 10:59:17 -0800 Message-ID: <87woomqy62.fsf@anholt.net> References: <20181205164529.12791-1-boris.brezillon@bootlin.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1938269319==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 5AE1D6E634 for ; Thu, 6 Dec 2018 18:59:20 +0000 (UTC) In-Reply-To: <20181205164529.12791-1-boris.brezillon@bootlin.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Cc: Boris Brezillon , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1938269319== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Boris Brezillon writes: > Commit 3e407417b192 ("drm/vc4: Fix X/Y positioning of planes using > T_TILES modifier") fixed the problem with T_TILES format, but left > things in a non-working state for SAND formats. Address that now. > > Signed-off-by: Boris Brezillon > --- > Hi Eric, > > So, I finally spent time debugging my SANDXXX pattern generator and > could validate that negative X/Y positioning does not work (which I > was expecting :-)). The fix turns out to be simpler than I thought > (much simpler than on T-tiles), and we now have negative X/Y > positioning working on all kind of formats. > > Regards, > > Boris > > Changes in v2: > - New patch > --- > drivers/gpu/drm/vc4/vc4_plane.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_pl= ane.c > index 75db62cbe468..3132f5e1d16a 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -595,6 +595,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, > case DRM_FORMAT_MOD_BROADCOM_SAND128: > case DRM_FORMAT_MOD_BROADCOM_SAND256: { > uint32_t param =3D fourcc_mod_broadcom_param(fb->modifier); > + u32 tile_w, tile, x_off, pix_per_tile; >=20=20 > /* Column-based NV12 or RGBA. > */ > @@ -614,12 +615,15 @@ static int vc4_plane_mode_set(struct drm_plane *pla= ne, > switch (base_format_mod) { > case DRM_FORMAT_MOD_BROADCOM_SAND64: > tiling =3D SCALER_CTL0_TILING_64B; > + tile_w =3D 64; > break; > case DRM_FORMAT_MOD_BROADCOM_SAND128: > tiling =3D SCALER_CTL0_TILING_128B; > + tile_w =3D 128; > break; > case DRM_FORMAT_MOD_BROADCOM_SAND256: > tiling =3D SCALER_CTL0_TILING_256B_OR_T; > + tile_w =3D 256; > break; > default: > break; > @@ -630,6 +634,28 @@ static int vc4_plane_mode_set(struct drm_plane *plan= e, > return -EINVAL; > } >=20=20 > + pix_per_tile =3D tile_w / fb->format->cpp[0]; > + tile =3D vc4_state->src_x / pix_per_tile; > + x_off =3D vc4_state->src_x % pix_per_tile; > + > + /* Adjust the base pointer to the first pixel to be scanned > + * out. > + */ > + for (i =3D 0; i < num_planes; i++) { > + vc4_state->offsets[i] +=3D param * tile_w * tile; > + vc4_state->offsets[i] +=3D (vc4_state->src_y / > + (i ? v_subsample : 1)) * > + tile_w; > + } > + > + /* > + * SCALER_PITCH0_SINK_PIX does not seem to work for SAND > + * formats. Specify a negative START_X instead, even if it's > + * less efficient. > + */ > + if (x_off) > + vc4_state->crtc_x =3D -x_off; Wait. If we were supposed to start at a nonzero x position within the FB, then we instead put the image off the left hand side of the screen? That seems wrong. Did you test if we can just vc4_state->offsets[i] +=3D x_off * cpp? > + > pitch0 =3D VC4_SET_FIELD(param, SCALER_TILE_HEIGHT); > break; > } > @@ -655,7 +681,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, > vc4_state->pos0_offset =3D vc4_state->dlist_count; > vc4_dlist_write(vc4_state, > VC4_SET_FIELD(state->alpha >> 8, SCALER_POS0_FIXED_ALPHA) | > - VC4_SET_FIELD(vc4_state->crtc_x, SCALER_POS0_START_X) | > + VC4_SET_FIELD(vc4_state->crtc_x & SCALER_POS0_START_X_MASK, > + SCALER_POS0_START_X) | > VC4_SET_FIELD(vc4_state->crtc_y, SCALER_POS0_START_Y)); >=20=20 > /* Position Word 1: Scaled Image Dimensions. */ > --=20 > 2.17.1 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlwJcYUACgkQtdYpNtH8 nui2ww/+MekbdjA9okTETmM3f2NVCh1BobjKBiP5s5u/au0HNi2L3kc4sndQXnkc 17pkejPlOMg2wcj/ESosrOzBFnawYjGiF+ofAT0jpk+zVGF+MhKdeLRknlUu09Nj GNzMAM1klt2SJWnHzMzJJGUUNYaPi/JVgTublr24s0jfksVbB6+tbUQQbT45wmLG SeRAN8Jzy0W5VAmnhkZh6ip4sgrks/s/UOE7dNl7x+3Psmwean/UakzqSHbwKr3n EQho9xGx4IoWz3yP+wjHCzka+bPvFgTplenNOC9OA3vE4A/NKUtO/s0wrr0PXmr7 a4r5gNh9BIyHkGIh5IF7UVLJIx3rubNXNRr0S/P1kdPEZr1DAFlkJ23vQGaIclOt Wuo57c0OULht3DkhNB2yGnnuPb/cMPPPkDnmQMaO/WZaK7QfAJcbdyzUwGk62WSb 54DfybNGSl+4okPIL7ysg7uPzkl2Kbuev+Lsucdao3c4fY0YdtKWyUUuo/2Q/obb WaNAjRwM4SP2y0wsGOv+bTXgwxgVO21b4KB7nCHK+LkjVxov0oE2HuE2VVVm+Ap0 uql+/qHH6S3K/CgPlZlKZ5KvwNW2/fCb1IuFwTqnFA9fjVvgCF9e+nsPuZkNe+Xj pB9AewSoXh2fCM5ossFEfv08jnxRC4wTSqrLFydNPQDwJXGfv7c= =MQBp -----END PGP SIGNATURE----- --=-=-=-- --===============1938269319== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1938269319==--