From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhenyu Wang Subject: Re: [PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values Date: Tue, 20 Nov 2018 10:38:58 +0800 Message-ID: <20181120023858.GA12743@zhen-hp.sh.intel.com> References: <20181119144109.12994-1-imre.deak@intel.com> <20181119152926.GG9144@intel.com> <20181119185430.GB9568@ideak-desk.fi.intel.com> Reply-To: Zhenyu Wang Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0560554196==" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 26E656E2E5 for ; Tue, 20 Nov 2018 02:48:26 +0000 (UTC) In-Reply-To: <20181119185430.GB9568@ideak-desk.fi.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0560554196== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="nFreZHaLTZJo0R7j" Content-Disposition: inline --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2018.11.19 20:54:30 +0200, Imre Deak wrote: > On Mon, Nov 19, 2018 at 05:29:26PM +0200, Ville Syrj=E4l=E4 wrote: > > On Mon, Nov 19, 2018 at 04:41:07PM +0200, Imre Deak wrote: > > > Depending on the transcoder enum values to translate from transcoder > > > to pipe/transcoder register addresses can easily break if we add a new > > > transcoder. So remove the dependency by using named initializers. > > >=20 > > > Suggested-by: Ville Syrj=E4l=E4 > > > Cc: Ville Syrj=E4l=E4 > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/i915_pci.c | 52 ++++++++++++++++++++++++++++++-= ---------- > > > 1 file changed, 38 insertions(+), 14 deletions(-) > > >=20 > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i= 915_pci.c > > > index 983ae7fd8217..1b81d7cb209e 100644 > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > @@ -33,16 +33,30 @@ > > > #define GEN(x) .gen =3D (x), .gen_mask =3D BIT((x) - 1) > > > =20 > > > #define GEN_DEFAULT_PIPEOFFSETS \ > > > - .pipe_offsets =3D { PIPE_A_OFFSET, PIPE_B_OFFSET, \ > > > - PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \ > > > - .trans_offsets =3D { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ > > > - TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } > > > + .pipe_offsets =3D { \ > > > + [TRANSCODER_A] =3D PIPE_A_OFFSET, \ > > > + [TRANSCODER_B] =3D PIPE_B_OFFSET, \ > > > + [TRANSCODER_C] =3D PIPE_C_OFFSET, \ > > > + [TRANSCODER_EDP] =3D PIPE_EDP_OFFSET, \ > >=20 > > Hmm. We do define this as int pipe_offsets[I915_MAX_TRANSCODERS], and > > PIPECONF/TRANSCONF is per-transcoder so I suppose using TRANSCODER_foo > > here make sense. > >=20 > > Couple of things that came to mind while thinking about this: > > - pipe_offsets[] & co. could probably be u32 since we don't > > need negative values >=20 > Ok, can follow up with that. >=20 > > - _PIPE_EDP could be removed, which would maybe shrink a few > > arrays based on I915_MAX_PIPES >=20 > Agreed. Looks like all the users are in gvt: >=20 > - PIPECONF(_PIPE_EDP) should use PIPECONF(TRANSCODER_EDP) instead. The > current code would also break if we added a new transcoder. yeah, agreed. > - AFAICS > PIPEDSL(_PIPE_EDP) > PIPESTAT(_PIPE_EDP) > PIPE_FRMCOUNT_G4X(_PIPE_EDP) > PIPE_FLIPCOUNT_G4X(_PIPE_EDP) > are bogus, since these registers don't exist. > Thanks for pointing this out, looks they were added in very beginning, will remove those after double check. thanks > Adding gvt folks for these. >=20 > >=20 > > Patch is > > Reviewed-by: Ville Syrj=E4l=E4 > >=20 > > > + }, \ > > > + .trans_offsets =3D { \ > > > + [TRANSCODER_A] =3D TRANSCODER_A_OFFSET, \ > > > + [TRANSCODER_B] =3D TRANSCODER_B_OFFSET, \ > > > + [TRANSCODER_C] =3D TRANSCODER_C_OFFSET, \ > > > + [TRANSCODER_EDP] =3D TRANSCODER_EDP_OFFSET, \ > > > + } > > > =20 > > > #define GEN_CHV_PIPEOFFSETS \ > > > - .pipe_offsets =3D { PIPE_A_OFFSET, PIPE_B_OFFSET, \ > > > - CHV_PIPE_C_OFFSET }, \ > > > - .trans_offsets =3D { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ > > > - CHV_TRANSCODER_C_OFFSET } > > > + .pipe_offsets =3D { \ > > > + [TRANSCODER_A] =3D PIPE_A_OFFSET, \ > > > + [TRANSCODER_B] =3D PIPE_B_OFFSET, \ > > > + [TRANSCODER_C] =3D CHV_PIPE_C_OFFSET, \ > > > + }, \ > > > + .trans_offsets =3D { \ > > > + [TRANSCODER_A] =3D TRANSCODER_A_OFFSET, \ > > > + [TRANSCODER_B] =3D TRANSCODER_B_OFFSET, \ > > > + [TRANSCODER_C] =3D CHV_TRANSCODER_C_OFFSET, \ > > > + } > > > =20 > > > #define CURSOR_OFFSETS \ > > > .cursor_offsets =3D { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_= C_OFFSET } > > > @@ -592,12 +606,22 @@ static const struct intel_device_info intel_can= nonlake_info =3D { > > > =20 > > > #define GEN11_FEATURES \ > > > GEN10_FEATURES, \ > > > - .pipe_offsets =3D { PIPE_A_OFFSET, PIPE_B_OFFSET, \ > > > - PIPE_C_OFFSET, PIPE_EDP_OFFSET, \ > > > - PIPE_DSI0_OFFSET, PIPE_DSI1_OFFSET }, \ > > > - .trans_offsets =3D { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ > > > - TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \ > > > - TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \ > > > + .pipe_offsets =3D { \ > > > + [TRANSCODER_A] =3D PIPE_A_OFFSET, \ > > > + [TRANSCODER_B] =3D PIPE_B_OFFSET, \ > > > + [TRANSCODER_C] =3D PIPE_C_OFFSET, \ > > > + [TRANSCODER_EDP] =3D PIPE_EDP_OFFSET, \ > > > + [TRANSCODER_DSI_0] =3D PIPE_DSI0_OFFSET, \ > > > + [TRANSCODER_DSI_1] =3D PIPE_DSI1_OFFSET, \ > > > + }, \ > > > + .trans_offsets =3D { \ > > > + [TRANSCODER_A] =3D TRANSCODER_A_OFFSET, \ > > > + [TRANSCODER_B] =3D TRANSCODER_B_OFFSET, \ > > > + [TRANSCODER_C] =3D TRANSCODER_C_OFFSET, \ > > > + [TRANSCODER_EDP] =3D TRANSCODER_EDP_OFFSET, \ > > > + [TRANSCODER_DSI_0] =3D TRANSCODER_DSI0_OFFSET, \ > > > + [TRANSCODER_DSI_1] =3D TRANSCODER_DSI1_OFFSET, \ > > > + }, \ > > > GEN(11), \ > > > .ddb_size =3D 2048, \ > > > .has_logical_ring_elsq =3D 1 > > > --=20 > > > 2.13.2 > >=20 > > --=20 > > Ville Syrj=E4l=E4 > > Intel --=20 Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 --nFreZHaLTZJo0R7j Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EARECAB0WIQTXuabgHDW6LPt9CICxBBozTXgYJwUCW/NzwgAKCRCxBBozTXgY J09bAJ0W+6l0tIkqlseEAJI4F6dfz/63hwCbBAlormng9qVQdAEMPuLMf7zaGXI= =maSk -----END PGP SIGNATURE----- --nFreZHaLTZJo0R7j-- --===============0560554196== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============0560554196==--