All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gupta, Anshuman" <anshuman.gupta@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [RFC v2] drm/i915/pps: Add PPS power domain
Date: Thu, 7 Jan 2021 05:21:36 +0000	[thread overview]
Message-ID: <36d4065f86974146bd7f654d428a6faa@intel.com> (raw)
In-Reply-To: <20210106160236.GA140339@ideak-desk.fi.intel.com>



> -----Original Message-----
> From: Imre Deak <imre.deak@intel.com>
> Sent: Wednesday, January 6, 2021 9:33 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Mason, Michael W
> <michael.w.mason@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Ville
> Syrjälä <ville.syrjala@linux.intel.com>
> Subject: Re: [RFC v2] drm/i915/pps: Add PPS power domain
> 
> On Wed, Jan 06, 2021 at 10:04:38AM +0530, Anshuman Gupta wrote:
> > It abstracts getting the AUX power domain in pps_lock under PPS power
> > domain. This makes sure that the platforms which really requires AUX
> > power in order to access PPS registers will get the reference to
> > necessary power wells.
> >
> > PPS power domain requires only to track the AUX_A associated power
> > wells as the platforms need AUX power in order to access PPS registers
> > supports eDP only on PORT_A.
> >
> > v2:
> > - Fixed missed POWER_DOMAIN_PPS in pps_unlock().
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_power.c | 7 +++++++
> > drivers/gpu/drm/i915/display/intel_display_power.h | 1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c            | 7 ++-----
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index d52374f01316..1dc4ca9e5d1a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -107,6 +107,8 @@ intel_display_power_domain_str(enum
> intel_display_power_domain domain)
> >  		return "VGA";
> >  	case POWER_DOMAIN_AUDIO:
> >  		return "AUDIO";
> > +	case POWER_DOMAIN_PPS:
> > +		return "PPS";
> >  	case POWER_DOMAIN_AUX_A:
> >  		return "AUX_A";
> >  	case POWER_DOMAIN_AUX_B:
> > @@ -2651,11 +2653,13 @@
> intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
> >  	BIT_ULL(POWER_DOMAIN_GT_IRQ) |			\
> >  	BIT_ULL(POWER_DOMAIN_MODESET) |			\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > +	BIT_ULL(POWER_DOMAIN_PPS) |			\
> >  	BIT_ULL(POWER_DOMAIN_GMBUS) |			\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >  #define BXT_DPIO_CMN_A_POWER_DOMAINS (			\
> >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > +	BIT_ULL(POWER_DOMAIN_PPS) |			\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >  #define BXT_DPIO_CMN_BC_POWER_DOMAINS (			\
> >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> > @@ -2688,6 +2692,7 @@ intel_display_power_put_mask_in_set(struct
> drm_i915_private *i915,
> >  #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
> >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > +	BIT_ULL(POWER_DOMAIN_PPS) |			\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >  #define GLK_DPIO_CMN_B_POWER_DOMAINS (			\
> >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> > @@ -2700,6 +2705,7 @@ intel_display_power_put_mask_in_set(struct
> drm_i915_private *i915,
> >  #define GLK_DISPLAY_AUX_A_POWER_DOMAINS (		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_IO_A) |		\
> > +	BIT_ULL(POWER_DOMAIN_PPS) |			\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >  #define GLK_DISPLAY_AUX_B_POWER_DOMAINS (		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_B) |		\
> > @@ -2712,6 +2718,7 @@ intel_display_power_put_mask_in_set(struct
> drm_i915_private *i915,
> >  	BIT_ULL(POWER_DOMAIN_GT_IRQ) |			\
> >  	BIT_ULL(POWER_DOMAIN_MODESET) |			\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > +	BIT_ULL(POWER_DOMAIN_PPS) |			\
> >  	BIT_ULL(POWER_DOMAIN_GMBUS) |			\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index bc30c479be53..7642be3c8e2e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -55,6 +55,7 @@ enum intel_display_power_domain {
> >  	POWER_DOMAIN_PORT_OTHER,
> >  	POWER_DOMAIN_VGA,
> >  	POWER_DOMAIN_AUDIO,
> > +	POWER_DOMAIN_PPS,
> >  	POWER_DOMAIN_AUX_A,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 8a00e609085f..99b4bec3c926 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -895,8 +895,7 @@ pps_lock(struct intel_dp *intel_dp)
> >  	 * See intel_power_sequencer_reset() why we need
> >  	 * a power domain reference here.
> >  	 */
> > -	wakeref = intel_display_power_get(dev_priv,
> > -
> intel_aux_power_domain(dp_to_dig_port(intel_dp)));
> > +	wakeref = intel_display_power_get(dev_priv,
> POWER_DOMAIN_PPS);
> 
> The purpose of this reference is to prevent a race when resetting the PPS
> state (see the comment), so I think we could reuse
> POWER_DOMAIN_DISPLAY_CORE instead of adding a new domain.
Thanks Imre for review, I will send a new patch with the fix.
Thanks,
Anshuman Gupta.
> 
> >
> >  	mutex_lock(&dev_priv->pps_mutex);
> >
> > @@ -909,9 +908,7 @@ pps_unlock(struct intel_dp *intel_dp,
> intel_wakeref_t wakeref)
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >
> >  	mutex_unlock(&dev_priv->pps_mutex);
> > -	intel_display_power_put(dev_priv,
> > -
> 	intel_aux_power_domain(dp_to_dig_port(intel_dp)),
> > -				wakeref);
> > +	intel_display_power_put(dev_priv, POWER_DOMAIN_PPS,
> wakeref);
> >  	return 0;
> >  }
> >
> > --
> > 2.26.2
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-01-07  5:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-05  5:13 [Intel-gfx] [RFC] drm/i915/pps: Add PPS power domain Anshuman Gupta
2021-01-05  6:24 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for " Patchwork
2021-01-06  4:34 ` [Intel-gfx] [RFC v2] " Anshuman Gupta
2021-01-06 16:02   ` Imre Deak
2021-01-07  5:21     ` Gupta, Anshuman [this message]
2021-01-07 11:23   ` [Intel-gfx] [PATCH] drm/i915/pps: Reuse POWER_DOMAIN_DISPLAY_CORE in pps_{lock, unlock} Anshuman Gupta
2021-01-06  5:29 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pps: Add PPS power domain (rev2) Patchwork
2021-01-06  8:13 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-01-06  8:36 ` [Intel-gfx] [drm/i915/pps] 46f952d34b: WARNING:at_drivers/gpu/drm/i915/display/intel_display_power.c:#__intel_display_power_put_async[i915] kernel test robot
2021-01-06  8:36   ` kernel test robot
2021-01-06  8:36   ` kernel test robot
2021-01-07 17:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pps: Add PPS power domain (rev3) Patchwork
2021-01-07 22:21 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=36d4065f86974146bd7f654d428a6faa@intel.com \
    --to=anshuman.gupta@intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.