| Message ID | 20181005232306.31133-20-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression enabling on eDP/DP | expand |
Hi Imre/Ville, This patch adds the power domain as per our discussion and feedback on previous patch set. Could you please take a look at this? Manasi 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, > > POWER_DOMAIN_NUM, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 3cf8533e0834..3ed0a3a1015a 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "MODESET"; > case POWER_DOMAIN_GT_IRQ: > return "GT_IRQ"; > + case POWER_DOMAIN_VDSC_PIPE_A: > + return "VDSC_PIPE_A"; > default: > MISSING_CASE(domain); > return "?"; > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > */ > #define ICL_PW_2_POWER_DOMAINS ( \ > ICL_PW_3_POWER_DOMAINS | \ > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > /* > - * - eDP/DSI VDSC > * - KVMR (HW control) > */ > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > -- > 2.18.0 >
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. > > POWER_DOMAIN_NUM, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 3cf8533e0834..3ed0a3a1015a 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "MODESET"; > case POWER_DOMAIN_GT_IRQ: > return "GT_IRQ"; > + case POWER_DOMAIN_VDSC_PIPE_A: > + return "VDSC_PIPE_A"; > default: > MISSING_CASE(domain); > return "?"; > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > */ > #define ICL_PW_2_POWER_DOMAINS ( \ > ICL_PW_3_POWER_DOMAINS | \ > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > /* > - * - eDP/DSI VDSC > * - KVMR (HW control) > */ > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > -- > 2.18.0
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. The only slightly confusing piece would be intel_display_power_domain_str() as that would have to reuturn a string with both names perhaps? > > > > > POWER_DOMAIN_NUM, > > }; > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 3cf8533e0834..3ed0a3a1015a 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > return "MODESET"; > > case POWER_DOMAIN_GT_IRQ: > > return "GT_IRQ"; > > + case POWER_DOMAIN_VDSC_PIPE_A: > > + return "VDSC_PIPE_A"; > > default: > > MISSING_CASE(domain); > > return "?"; > > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > */ > > #define ICL_PW_2_POWER_DOMAINS ( \ > > ICL_PW_3_POWER_DOMAINS | \ > > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > /* > > - * - eDP/DSI VDSC > > * - KVMR (HW control) > > */ > > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > -- > > 2.18.0 > > -- > Ville Syrjälä > Intel
Thanks for your review comments. 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. Okay will make these changes in the next rev. With these changes, can I consider your r-b? Regards Manasi > > > > > POWER_DOMAIN_NUM, > > }; > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 3cf8533e0834..3ed0a3a1015a 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > return "MODESET"; > > case POWER_DOMAIN_GT_IRQ: > > return "GT_IRQ"; > > + case POWER_DOMAIN_VDSC_PIPE_A: > > + return "VDSC_PIPE_A"; > > default: > > MISSING_CASE(domain); > > return "?"; > > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > */ > > #define ICL_PW_2_POWER_DOMAINS ( \ > > ICL_PW_3_POWER_DOMAINS | \ > > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > /* > > - * - eDP/DSI VDSC > > * - KVMR (HW control) > > */ > > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > -- > > 2.18.0 > > -- > Ville Syrjälä > Intel
On Tue, Oct 16, 2018 at 12:19:24PM -0700, Manasi Navare wrote: > Thanks for your review comments. > > 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. > > Okay will make these changes in the next rev. > With these changes, can I consider your r-b? The slight rename+move I'd like to see. I'll leave it up to you whether to pursue the aliasing TRANSCODER_EDP_VDSC idea. > > Regards > Manasi > > > > > > > > > POWER_DOMAIN_NUM, > > > }; > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 3cf8533e0834..3ed0a3a1015a 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > return "MODESET"; > > > case POWER_DOMAIN_GT_IRQ: > > > return "GT_IRQ"; > > > + case POWER_DOMAIN_VDSC_PIPE_A: > > > + return "VDSC_PIPE_A"; > > > default: > > > MISSING_CASE(domain); > > > return "?"; > > > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > */ > > > #define ICL_PW_2_POWER_DOMAINS ( \ > > > ICL_PW_3_POWER_DOMAINS | \ > > > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > /* > > > - * - eDP/DSI VDSC > > > * - KVMR (HW control) > > > */ > > > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > -- > > > 2.18.0 > > > > -- > > Ville Syrjälä > > Intel
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? Manasi > The only slightly confusing piece would be > intel_display_power_domain_str() as that would have to reuturn > a string with both names perhaps? > > > > > > > > > POWER_DOMAIN_NUM, > > > }; > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 3cf8533e0834..3ed0a3a1015a 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > > > return "MODESET"; > > > case POWER_DOMAIN_GT_IRQ: > > > return "GT_IRQ"; > > > + case POWER_DOMAIN_VDSC_PIPE_A: > > > + return "VDSC_PIPE_A"; > > > default: > > > MISSING_CASE(domain); > > > return "?"; > > > @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, > > > */ > > > #define ICL_PW_2_POWER_DOMAINS ( \ > > > ICL_PW_3_POWER_DOMAINS | \ > > > + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ > > > BIT_ULL(POWER_DOMAIN_INIT)) > > > /* > > > - * - eDP/DSI VDSC > > > * - KVMR (HW control) > > > */ > > > #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \ > > > -- > > > 2.18.0 > > > > -- > > 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
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, ... };
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
On Tue, Oct 16, 2018 at 02:04:21PM -0700, Manasi Navare wrote: > 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 > Ok final consensus is to name it as POWER_DOMAIN_TRANSCODER_EDP_VDSC as per the IRC discusion. 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
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, POWER_DOMAIN_NUM, }; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 3cf8533e0834..3ed0a3a1015a 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -146,6 +146,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "MODESET"; case POWER_DOMAIN_GT_IRQ: return "GT_IRQ"; + case POWER_DOMAIN_VDSC_PIPE_A: + return "VDSC_PIPE_A"; default: MISSING_CASE(domain); return "?"; @@ -1971,9 +1973,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, */ #define ICL_PW_2_POWER_DOMAINS ( \ ICL_PW_3_POWER_DOMAINS | \ + BIT_ULL(POWER_DOMAIN_VDSC_PIPE_A) | \ BIT_ULL(POWER_DOMAIN_INIT)) /* - * - eDP/DSI VDSC * - KVMR (HW control) */ #define ICL_DISPLAY_DC_OFF_POWER_DOMAINS ( \
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(-)