All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [CI] drm/i915/cnl: New power domain for AUX IO.
Date: Mon, 19 Feb 2018 17:49:41 +0200	[thread overview]
Message-ID: <20180219154941.GW5453@intel.com> (raw)
In-Reply-To: <20180219144741.jdro2kjovxdrym72@ideak-desk.fi.intel.com>

On Mon, Feb 19, 2018 at 04:47:41PM +0200, Imre Deak wrote:
> On Fri, Feb 16, 2018 at 04:27:04PM -0800, Rodrigo Vivi wrote:
> > On Fri, Feb 16, 2018 at 03:25:55PM -0800, Dhinakaran Pandiyan wrote:
> > > PSR requires AUX IO well to be kept on and the existing AUX domain
> > > enables other wells too.
> > > 
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.h    |  5 +++++
> > >  drivers/gpu/drm/i915/intel_dp.c         |  6 ++++++
> > >  drivers/gpu/drm/i915/intel_drv.h        |  1 +
> > >  drivers/gpu/drm/i915/intel_psr.c        |  4 ++++
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++
> > >  5 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index c4042e342f50..9857b60a7e0f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -175,6 +175,11 @@ enum intel_display_power_domain {
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_AUX_D,
> > >  	POWER_DOMAIN_AUX_F,
> > > +	POWER_DOMAIN_AUX_IO_A,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > > +	POWER_DOMAIN_AUX_IO_D,
> > > +	POWER_DOMAIN_AUX_IO_F,
> > 
> > I know that I had suggested something very similar, but looking now
> > to the code it seems a big duplication of all other aux..
> > 
> > I mean, on CNL
> > 
> > POWER_DOMAIN_AUX_{B,C,D,F} are exactly the same as
> > POWER_DOMAIN_AUX__IO_{B,C,D,F}
> > 
> > Only the AUX_IO A is different from the AUX_A because AUX_A
> > disables DMC.
> > So, question is: What about removing the AUX_A domain from DC_OFF
> > and just using AUX doamins... Well, on this case calling get/put
> > only for INTEL_GEN <= 10...
> > 
> > So, yeap, not beautiful anyways.
> > 
> > I'd like to hear from Imre what he had in mind for handling
> > AUX IOs and to know if he has any better idea for this case.
> 
> Hm, so in general (for AUX transfers) to enable AUX-A we first need to
> disable DC states _except_ if we enable AUX-A for PSR where we want
> DC5/6 to stay enabled. Nice. Also not sure why the FW/HW can't have its
> own request bit..
> 
> I don't have a better idea, but since on DDI where this matters PSR is
> tied to port A,

SKL+ (or maybe even BDW?) can do PSR on any port actually. We just don't
make use of that capability currently.

> we could have only a POWER_DOMAIN_PSR and map it to the
> AUX-A power well where it's needed.
> 
> --Imre
> 
> > 
> > >  	POWER_DOMAIN_GMBUS,
> > >  	POWER_DOMAIN_MODESET,
> > >  	POWER_DOMAIN_GT_IRQ,
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index f20b25f98e5a..aa90f6a70689 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -6280,22 +6280,28 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> > >  	switch (encoder->port) {
> > >  	case PORT_A:
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_A;
> > >  		break;
> > >  	case PORT_B:
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_B;
> > >  		break;
> > >  	case PORT_C:
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_C;
> > >  		break;
> > >  	case PORT_D:
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D;
> > >  		break;
> > >  	case PORT_E:
> > >  		/* FIXME: Check VBT for actual wiring of PORT E */
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_D;
> > >  		break;
> > >  	case PORT_F:
> > >  		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_F;
> > > +		intel_dp->aux_io_power_domain = POWER_DOMAIN_AUX_IO_F;
> > >  		break;
> > >  	default:
> > >  		MISSING_CASE(encoder->port);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 898064e8bea7..c7c4180e41d8 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1071,6 +1071,7 @@ struct intel_dp {
> > >  	struct drm_dp_desc desc;
> > >  	struct drm_dp_aux aux;
> > >  	enum intel_display_power_domain aux_power_domain;
> > > +	enum intel_display_power_domain aux_io_power_domain;
> > >  	uint8_t train_set[4];
> > >  	int panel_power_up_delay;
> > >  	int panel_power_down_delay;
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index 2ef374f936b9..62de8f0632b0 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -484,6 +484,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> > >  			   EDP_PSR_DEBUG_MASK_HPD |
> > >  			   EDP_PSR_DEBUG_MASK_LPSP);
> > >  	}
> > > +
> > > +	intel_display_power_get(dev_priv, intel_dp->aux_io_power_domain);
> > >  }
> > >  
> > >  /**
> > > @@ -617,6 +619,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> > >  		else
> > >  			WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> > >  	}
> > > +
> > > +	intel_display_power_put(dev_priv, intel_dp->aux_io_power_domain);
> > >  }
> > >  
> > >  /**
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 16790f2576ec..323c1babb0a0 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -130,6 +130,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > >  		return "AUX_D";
> > >  	case POWER_DOMAIN_AUX_F:
> > >  		return "AUX_F";
> > > +	case POWER_DOMAIN_AUX_IO_A:
> > > +		return "AUX_IO_A";
> > > +	case POWER_DOMAIN_AUX_IO_B:
> > > +		return "AUX_IO_B";
> > > +	case POWER_DOMAIN_AUX_IO_C:
> > > +		return "AUX_IO_C";
> > > +	case POWER_DOMAIN_AUX_IO_D:
> > > +		return "AUX_IO_D";
> > > +	case POWER_DOMAIN_AUX_IO_F:
> > > +		return "AUX_IO_F";
> > >  	case POWER_DOMAIN_GMBUS:
> > >  		return "GMBUS";
> > >  	case POWER_DOMAIN_INIT:
> > > @@ -1853,18 +1863,23 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_AUX_A_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_IO_A) |		\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_AUX_B_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_B) |			\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_IO_B) |		\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_AUX_C_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_C) |			\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_IO_C) |		\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_AUX_D_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_D) |			\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_IO_D) |		\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_AUX_F_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_F) |			\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_IO_F) |		\
> > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > >  #define CNL_DISPLAY_DDI_F_IO_POWER_DOMAINS (		\
> > >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_F_IO) |		\
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

  reply	other threads:[~2018-02-19 15:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-16 23:25 [CI] drm/i915/cnl: New power domain for AUX IO Dhinakaran Pandiyan
2018-02-16 23:57 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-02-17  0:27 ` [CI] " Rodrigo Vivi
2018-02-19 14:47   ` Imre Deak
2018-02-19 15:49     ` Ville Syrjälä [this message]
2018-02-20 15:35       ` Imre Deak
2018-02-20 16:25         ` Imre Deak
2018-02-20 18:44           ` Rodrigo Vivi
2018-02-20 20:04             ` Pandiyan, Dhinakaran
2018-02-17  0:45 ` ✗ Fi.CI.IGT: failure for " 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=20180219154941.GW5453@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=dhinakaran.pandiyan@intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@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.