Re: [PATCH v5 19/28] 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 v5 19/28] drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI
Date: Tue, 16 Oct 2018 14:04:21 -0700	[thread overview]
Message-ID: <20181016210420.GF8224@intel.com> (raw)
In-Reply-To: <20181016194555.GP9144@intel.com>

On Tue, Oct 16, 2018 at 10:45:55PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 16, 2018 at 12:42:05PM -0700, Manasi Navare wrote:
> > On Tue, Oct 16, 2018 at 10:19:06PM +0300, Ville Syrjälä wrote:
> > > On Tue, Oct 16, 2018 at 10:01:11PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Oct 05, 2018 at 04:22:57PM -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.
> > > > > 
> > > > > v2:
> > > > > * Fix the power well mismatch CI error (Ville)
> > > > > * Rename as VDSC_PIPE_A (Imre)
> > > > > * Fix a whitespace (Anusha)
> > > > > * Fix Comments (Imre)
> > > > > 
> > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > > > 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 | 4 +++-
> > > > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > > index 9eaba1bccae8..4c513169960c 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_PIPE_A,
> > > > 
> > > > I'd probably put it next to the other pipe related power domains.
> > > > So maybe after POWER_DOMAIN_PIPE_C_PANEL_FITTER.
> > > > 
> > > > And to match the current naming pattern it should be called
> > > > POWER_DOMAIN_PIPE_A_VDSC.
> > > 
> > > Hmm. We could also give it an alias TRANSCODER_EDP_VDSC. Making
> > > it an alias would avoid wasting yet another bit, but would make
> > > the code easier to understand as we wouldn't have to add comments
> > > explaining why we use a PIPE_A_VDSC power domain based on the
> > > usage of the EDP transcoder.
> > >
> > 
> > So you are suggesting adding an alias TRANSCODER_EDP_VDSC for POWER_DOMAIN_PIPE_A_VDSC?
> > But how does it avoid wasting another bit, since we would still have POWER_DOMAIN_PIPE_A_VDSC as a field
> > in enum power domains right?
> 
> enum ... {
> 	...
> 	POWER_DOMAIN_PIPE_A_VDSC,
> 	POWER_DOMAIN_TRANSCODER_EDP_VDSC = POWER_DOMAIN_PIPE_A_VDSC,
> 	...
> };

Why keep the POWER_DOMAIN_PIPE_A_VDSC name at all? Just for using it for *something*..?

Manasi

> 
> -- 
> 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