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

From: Manasi Navare <manasi.d.navare@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v4 19/25] drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI
Date: Tue, 18 Sep 2018 14:10:17 -0700	[thread overview]
Message-ID: <20180918211017.GB20073@intel.com> (raw)
In-Reply-To: <20180918194646.GC5565@intel.com>

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@intel.com>
> > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@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
Can we have a situation where eDP uses Pipe B? 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx