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

[Manasi Navare]

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.
But for Pipe A only, it uses VDSC engine attached to transcoder eDP/DSI.
 
> > Can we have a situation where eDP uses Pipe B?
> 
> Sure. 'xrandr --output eDP --crtc 1', or whatever.
>

I guess even in this case, if its eDP/DSI, it will still use the VDSC engine
on transcoder eDP so will need to enable this power well 2.

Manasi
 
> > Because in case of VDSC
> > in case of eDP, it will always use the VDSC on transcoder eDP which will
> > require this power well.
> > 
> > Manasi
> > 
> > > 
> > > > So we could call it VDSC_PIPE_A since VDSC on Pipe A for eDP/DSI
> > > > will use this power well. But may be we should add a comment that
> > > > this is only for eDP/DSI on Pipe A since ICL does not support
> > > > VDSC on DP on Pipe A
> > > > 
> > > > What do you think?
> > > > 
> > > > Manasi
> > > > 
> > > > > 
> > > > > If we call it PIPE_A then it's going to a bit confusing when we
> > > > > use it with pipe B or C. Needs at least clear comments in the code
> > > > > why we're doing something that looks like nonsense of the first
> > > > > glance.
> > > > > 
> > > > > -- 
> > > > > Ville Syrjälä
> > > > > Intel
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel