Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Gustavo Sousa <gustavo.sousa@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Move hotplug inversion logic into separate helper
Date: Wed, 21 Sep 2022 12:59:22 +0300	[thread overview]
Message-ID: <87v8ph6oat.fsf@intel.com> (raw)
In-Reply-To: <20220920162742.3x6z2z6p24bkea6x@gjsousa-mobl2>

On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> On Tue, Sep 20, 2022 at 12:01:36AM -0700, Lucas De Marchi wrote:
>> On Mon, Sep 19, 2022 at 11:56:59AM -0300, Gustavo Sousa wrote:
>> > Make the code more readable, which will be more apparent as new
>> > platforms with different hotplug inversion needs are added.
>> > 
>> > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
>> > 1 file changed, 16 insertions(+), 9 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index de06f293e173..c53d21ae197f 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> > 	spin_unlock_irq(&dev_priv->irq_lock);
>> > }
>> > 
>> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
>> 
>> new users of drm_i915_private should use "i915" as variable name rather
>> than dev_priv.
>
> Thanks. I will update this.
>
> Is there any documentation where we can find information like this?

WIP:

https://gitlab.freedesktop.org/jani/i915-check/-/blob/main/i915-style-guide.rst

BR,
Jani.

>
>> 
>> other than that,  Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> 
>> 
>> Lucas De Marchi
>> 
>> > +{
>> > +	u32 invert_bits;
>> > +
>> > +	if (HAS_PCH_DG1(dev_priv))
>> > +		invert_bits = INVERT_DDIA_HPD |
>> > +			      INVERT_DDIB_HPD |
>> > +			      INVERT_DDIC_HPD |
>> > +			      INVERT_DDID_HPD;
>> > +	else
>> > +		return;
>> > +
>> > +	intel_uncore_rmw(&dev_priv->uncore, SOUTH_CHICKEN1, 0, invert_bits);
>> > +}
>> > +
>> > static u32 ibx_hotplug_enables(struct drm_i915_private *i915,
>> > 			       enum hpd_pin pin)
>> > {
>> > @@ -3413,15 +3428,7 @@ static u32 gen11_hotplug_enables(struct drm_i915_private *i915,
>> > 
>> > static void dg1_hpd_irq_setup(struct drm_i915_private *dev_priv)
>> > {
>> > -	u32 val;
>> > -
>> > -	val = intel_uncore_read(&dev_priv->uncore, SOUTH_CHICKEN1);
>> > -	val |= (INVERT_DDIA_HPD |
>> > -		INVERT_DDIB_HPD |
>> > -		INVERT_DDIC_HPD |
>> > -		INVERT_DDID_HPD);
>> > -	intel_uncore_write(&dev_priv->uncore, SOUTH_CHICKEN1, val);
>> > -
>> > +	setup_hotplug_inversion(dev_priv);
>> > 	icp_hpd_irq_setup(dev_priv);
>> > }
>> > 
>> > -- 
>> > 2.37.3
>> > 

-- 
Jani Nikula, Intel Open Source Graphics Center