Re: [Intel-gfx] [PATCH v3 2/3] drm/i915: Stash hpd status bits under dev_priv

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 2/3] drm/i915: Stash hpd status bits under dev_priv
Date: Thu, 7 May 2020 09:53:13 +0300	[thread overview]
Message-ID: <20200507065313.GX6112@intel.com> (raw)
In-Reply-To: <20200506160341.GB8813@ideak-desk.fi.intel.com>

On Wed, May 06, 2020 at 07:03:41PM +0300, Imre Deak wrote:
> On Wed, Mar 11, 2020 at 05:54:21PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Instead of constnantly having to figure out which hpd status bit
> > array to use let's store them under dev_priv.
> > 
> > Should perhaps take this further and stash even more stuff to
> > make the hpd handling more abstract yet.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |   2 +
> >  drivers/gpu/drm/i915/i915_irq.c | 198 ++++++++++++++++++--------------
> >  2 files changed, 111 insertions(+), 89 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 19195bde4921..b5afbabf4c3b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -149,6 +149,8 @@ enum hpd_pin {
> >  struct i915_hotplug {
> >  	struct delayed_work hotplug_work;
> >  
> > +	const u32 *hpd, *pch_hpd;
> > +
> >  	struct {
> >  		unsigned long last_jiffies;
> >  		int count;
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 9f0653cf0510..b95d952bd47d 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -124,7 +124,6 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> >  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS,
> >  };
> >  
> > -/* BXT hpd list */
> >  static const u32 hpd_bxt[HPD_NUM_PINS] = {
> >  	[HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
> >  	[HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> > @@ -168,6 +167,44 @@ static const u32 hpd_tgp[HPD_NUM_PINS] = {
> >  	[HPD_PORT_I] = SDE_TC_HOTPLUG_ICP(PORT_TC6),
> >  };
> >  
> > +static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
> > +{
> > +	struct i915_hotplug *hpd = &dev_priv->hotplug;
> > +
> > +	if (HAS_GMCH(dev_priv)) {
> > +		if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
> > +		    IS_CHERRYVIEW(dev_priv))
> > +			hpd->hpd = hpd_status_g4x;
> > +		else
> > +			hpd->hpd = hpd_status_i915;
> > +		return;
> > +	}
> > +
> > +	if (INTEL_GEN(dev_priv) >= 12)
> > +		hpd->hpd = hpd_gen12;
> > +	else if (INTEL_GEN(dev_priv) >= 11)
> > +		hpd->hpd = hpd_gen11;
> > +	else if (IS_GEN9_LP(dev_priv))
> > +		hpd->hpd = hpd_bxt;
> > +	else if (INTEL_GEN(dev_priv) >= 8)
> > +		hpd->hpd = hpd_bdw;
> > +	else if (INTEL_GEN(dev_priv) >= 7)
> > +		hpd->hpd = hpd_ivb;
> > +	else
> > +		hpd->hpd = hpd_ilk;
> > +
> > +	if (HAS_PCH_TGP(dev_priv) || HAS_PCH_JSP(dev_priv))
> > +		hpd->pch_hpd = hpd_tgp;
> > +	else if (HAS_PCH_ICP(dev_priv) || HAS_PCH_MCC(dev_priv))
> > +		hpd->pch_hpd = hpd_icp;
> > +	else if (HAS_PCH_SPT(dev_priv))
> 
> What about CNP?

Argh. We have too many of these. Do we even need all of them?

> 
> > +		hpd->pch_hpd = hpd_spt;
> > +	else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_CPT(dev_priv))
> > +		hpd->pch_hpd = hpd_cpt;
> > +	else if (HAS_PCH_IBX(dev_priv))
> > +		hpd->pch_hpd = hpd_ibx;
> 
> Can we add MISSING_CASE for PCH platforms?

else if (HAS_PCH_SPLIT())
	MISSING_CASE(INTEL_PCH_TYPE())
?

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx