[Intel-gfx] [PATCH v4 19/25] drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI

[Imre Deak]

On Fri, Sep 21, 2018 at 04:46:47PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 21, 2018 at 01:34:00AM -0700, Manasi Navare wrote:
> > On Wed, Sep 19, 2018 at 01:57:00PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 18, 2018 at 02:10:17PM -0700, Manasi Navare wrote:
> > > > On Tue, Sep 18, 2018 at 10:46:46PM +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 18, 2018 at 12:31:54PM -0700, Manasi Navare wrote:
> > > > > > On Tue, Sep 18, 2018 at 10:12:24PM +0300, Ville Syrjälä wrote:
> > > > > > > On Tue, Sep 18, 2018 at 12:04:35PM -0700, Manasi Navare wrote:
> > > > > > > > Thanks Imre for your review comments. Please find the comments below:
> > > > > > > > 
> > > > > > > > On Fri, Sep 14, 2018 at 01:55:00PM +0300, Imre Deak wrote:
> > > > > > > > > On Tue, Sep 11, 2018 at 05:56:01PM -0700, Manasi Navare wrote:
> > > > > > > > > > On Icelake, a separate power well PG2 is created for
> > > > > > > > > > VDSC engine used for eDP/MIPI DSI. This patch adds a new
> > > > > > > > > > display power domain for Power well 2.
> > > > > > > > > > 
> > > > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > > > > > > > Cc: Imre Deak <imre.deak at intel.com>
> > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/gpu/drm/i915/intel_display.h    |  1 +
> > > > > > > > > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
> > > > > > > > > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > > > index 3fe52788b4cf..bef71d27cdfe 100644
> > > > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > > > > > > > @@ -256,6 +256,7 @@ enum intel_display_power_domain {
> > > > > > > > > >  	POWER_DOMAIN_MODESET,
> > > > > > > > > >  	POWER_DOMAIN_GT_IRQ,
> > > > > > > > > >  	POWER_DOMAIN_INIT,
> > > > > > > > > > +	POWER_DOMAIN_VDSC_EDP_MIPI,
> > > > > > > > > 
> > > > > > > > > This is better named VDSC_PIPE_A. The other pipes have also VDSC
> > > > > > > > > functionality which could be on separate power wells in the future.
> > > > > > > > >
> > > > > > > > 
> > > > > > > > Yea naming it as VDSC_PIPE_A makes sense since eDP/MIPI DSI on Pipe A
> > > > > > > > will use this VDSC power well.
> > > > > > > > I will change this in the next revision.
> > > > > > > 
> > > > > > > Isn't the VDSC in the transcoder for now though? And I guess it's
> > > > > > > moving to the pipe later?
> > > > > > 
> > > > > > VDSC engine is attached to the eDP/DSI transcoders and this gets used
> > > > > > for eDP/DSI VDSC on Pipe A.
> > > > > 
> > > > > And what happens when I want to use pipe B instead?
> > > > 
> > > > DP VDSC on Pipe B uses the VDSC engine on Pipe B. Same for Pipe C
> > > 
> > > There are no VDSCs in pipe B or C. There are VDSCs in transcoder B
> > > and C. But that's not the same thing at all. The mux is between the
> > > pipe and transcoder.
> > >
> > 
> > As per the display overview for Gen 11, the VDSC engine is present on Pipe B And C.
> 
> On transcoder B and C, not pipe B and C.

Yep, I was wrong, the original name POWER_DOMAIN_VDSC_EDP_MIPI is ok.

Up to GEN11 pipe B,C use their associated pipe compression
engines/joiner if routed to transcoder B,C but they use the separate
compression engine (w/o a joiner) if routed to the eDP/MIPI transcoder.

One unclear thing is that the BSpec DSS_CTL1/2 register descriptions
(used for the eDP/MIPI DSC) show that they are backed by PG1, not PG2 as
implied elsewhere in the spec and in this patch.

Art, is that incorrect, or the registers are backed by a different power
well (PG1) than the functionality itself (PG2)?

--Imre