From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 6/7] drm/i915: vlv: fix mapping of pipestat enable to status bits Date: Wed, 05 Feb 2014 17:04:50 +0200 Message-ID: <1391612690.30971.43.camel@intelbox> References: <1391542551-20239-1-git-send-email-imre.deak@intel.com> <1391542551-20239-7-git-send-email-imre.deak@intel.com> <20140205145437.GK3891@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1792264594==" Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id AA7B8106EDD for ; Wed, 5 Feb 2014 07:04:53 -0800 (PST) In-Reply-To: <20140205145437.GK3891@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1792264594== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-H0BN+2s8ResWUpAutHq0" --=-H0BN+2s8ResWUpAutHq0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2014-02-05 at 16:54 +0200, Ville Syrj=C3=A4l=C3=A4 wrote: > On Tue, Feb 04, 2014 at 09:35:50PM +0200, Imre Deak wrote: > > At least on VLV we can't get at the pipestat status bits by simply righ= t > > shifting the corresponding enable bits. The mapping between enable and > > status bits for the sprite0,1 flip done and the PSR events don't follow > > this rule, so we need to map them separately. > >=20 > > The PSR enable for pipe A is DPFLIPSTAT[22], but I haven't added suppor= t > > for this, since there is no user of it atm. Until support is added WARN > > if someone tries to enable PSR interrupts, or tries to enable the same > > (1 << 6) bit on pipe B, which MBZ. > >=20 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++= ++- > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > 2 files changed, 38 insertions(+), 1 deletion(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i91= 5_irq.c > > index ec56a70..eea5398 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -471,9 +471,29 @@ done: > > return ret; > > } > > =20 > > +static void set_pipestat_enable_bit(u32 status_mask, u32 status_bit, > > + u32 enable_bit, u32 *enable_mask) > > +{ > > + if (status_mask & status_bit) > > + *enable_mask |=3D enable_bit; > > + else > > + *enable_mask &=3D ~enable_bit; > > +} > > + > > static u32 pipestat_to_enable_mask(struct drm_device *dev, u32 status_= mask) > > { > > - return status_mask << 16; > > + u32 enable_mask =3D status_mask << 16; > > + > > + if (!IS_VALLEYVIEW(dev)) > > + return enable_mask; > > + > > + enable_mask &=3D ~PIPE_FIFO_UNDERRUN_STATUS; > > + set_pipestat_enable_bit(status_mask, SPRITE1_FLIP_DONE_INT_STATUS_VLV= , > > + SPRITE1_FLIP_DONE_INT_EN_VLV, &enable_mask); > > + set_pipestat_enable_bit(enable_mask, SPRITE0_FLIP_DONE_INT_STATUS_VLV= , > > + SPRITE0_FLIP_DONE_INT_EN_VLV, &enable_mask); >=20 > This feels a bit subtle to me. Maybe do it in a bit more straightforward > way? Eg.: >=20 > enable_mask &=3D ~(PIPE_FIFO_UNDERRUN_STATUS | > SPRITE0_FLIP_DONE_INT_EN_VLV | > SPRITE1_FLIP_DONE_INT_EN_VLV); > if (status_mask & SPRITE0_FLIP_DONE_INT_STATUS_VLV) > enable_mask |=3D SPRITE0_FLIP_DONE_INT_EN_VLV; > if (status_mask & SPRITE1_FLIP_DONE_INT_STATUS_VLV) > enable_mask |=3D SPRITE1_FLIP_DONE_INT_EN_VLV; I added the helpers in case more remappings are needed. But since we have only the above two (and hopefully won't have more) I agree this is simpler, so I'll rewrite it. > > + > > + return enable_mask; > > } > > =20 > > void > > @@ -489,6 +509,13 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv,= enum pipe pipe, > > if (WARN_ON_ONCE(status_mask & ~PIPESTAT_INT_STATUS_MASK)) > > return; > > =20 > > + /* > > + * On pipe A we don't support the PSR interrupt yet, on pipe B the > > + * same bit MBZ. > > + */ > > + if (WARN_ON_ONCE(status_mask & PIPE_A_PSR_STATUS_VLV)) > > + return; >=20 > Won't this break the BLC interrupt on gen3/4? Good catch and me not testing this on those gens.. Will use instead here and in i915_disable_pipestat: if (WARN_ON_ONCE(IS_VALLEYVIEW(dev) && (status_mask & PIPE_A_PSR_STATUS_VLV))) return; --Imre > > + > > enable_mask =3D pipestat_to_enable_mask(dev_priv->dev, status_mask); > > if ((pipestat & enable_mask) =3D=3D enable_mask) > > return; > > @@ -512,6 +539,13 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv= , enum pipe pipe, > > if (WARN_ON_ONCE(status_mask & ~PIPESTAT_INT_STATUS_MASK)) > > return; > > =20 > > + /* > > + * On pipe A we don't support the PSR interrupt yet, on pipe B the > > + * same bit MBZ. > > + */ > > + if (WARN_ON_ONCE(status_mask & PIPE_A_PSR_STATUS_VLV)) > > + return; > > + > > enable_mask =3D pipestat_to_enable_mask(dev_priv->dev, status_mask); > > if ((pipestat & enable_mask) =3D=3D 0) > > return; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index 599c7f6..c998c4f 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3240,6 +3240,7 @@ > > #define PIPE_LEGACY_BLC_EVENT_ENABLE (1UL<<22) > > #define PIPE_ODD_FIELD_INTERRUPT_ENABLE (1UL<<21) > > #define PIPE_EVEN_FIELD_INTERRUPT_ENABLE (1UL<<20) > > +#define PIPE_B_PSR_INTERRUPT_ENABLE_VLV (1UL<<19) > > #define PIPE_HOTPLUG_TV_INTERRUPT_ENABLE (1UL<<18) /* pre-965 */ > > #define PIPE_START_VBLANK_INTERRUPT_ENABLE (1UL<<18) /* 965 or later= */ > > #define PIPE_VBLANK_INTERRUPT_ENABLE (1UL<<17) > > @@ -3256,8 +3257,10 @@ > > #define PIPE_DISPLAY_LINE_COMPARE_STATUS (1UL<<8) > > #define PIPE_DPST_EVENT_STATUS (1UL<<7) > > #define PIPE_LEGACY_BLC_EVENT_STATUS (1UL<<6) > > +#define PIPE_A_PSR_STATUS_VLV (1UL<<6) > > #define PIPE_ODD_FIELD_INTERRUPT_STATUS (1UL<<5) > > #define PIPE_EVEN_FIELD_INTERRUPT_STATUS (1UL<<4) > > +#define PIPE_B_PSR_STATUS_VLV (1UL<<3) > > #define PIPE_HOTPLUG_TV_INTERRUPT_STATUS (1UL<<2) /* pre-965 */ > > #define PIPE_START_VBLANK_INTERRUPT_STATUS (1UL<<2) /* 965 or later = */ > > #define PIPE_VBLANK_INTERRUPT_STATUS (1UL<<1) > > --=20 > > 1.8.4 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-H0BN+2s8ResWUpAutHq0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAABAgAGBQJS8lMSAAoJEORIIAnNuWDF/dIH/3Tb+USfPQtcK3FeXGBvMmer EMRGtxKMnQ7qDCqvSNr26U2qDnl+hIkjpadjGfz2Pr5iklNDZXOzd7NM7f0TYzx/ I4BkWTdiVC2JPBT57PvQWfkviNVP4R7AgmTgte5iZ/S+Zoj9vb75YWYcmuBI/QoB ldt8SXa+SCxfJxORcgcHTKlvnIrvDibAN8qivYkRNYuyruZTKyaBZ+qJqv44bg+1 QnB3ksGl7mC56mu2anfQtGPifphMw+ZbW95KnCAxGGlZnyfyxemV1GJfnON0EW7Z lKoI4baNQg98hsQ3vSTd87u1FMWIB59xKA5WwRA2P1SUSM2Jp0AIWb8q033RRbc= =HLXR -----END PGP SIGNATURE----- --=-H0BN+2s8ResWUpAutHq0-- --===============1792264594== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1792264594==--