| Message ID | 20200124084456.2961-4-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable second DBuf slice for ICL and TGL | expand |
On Fri, Jan 24, 2020 at 10:44:52AM +0200, Stanislav Lisovskiy wrote: > Now start using parameterized DBUF_CTL instead > of hardcoded, this would allow shorter access > functions when reading or storing entire state. > > Tried to implement it in a MMIO_PIPE manner, however > DBUF_CTL1 address is higher than DBUF_CTL2, which > implies that we have to now subtract from base > rather than add. > > v2: - Removed unneeded DBUF_CTL_DIST and DBUF_CTL_ADDR > macros. Started to use _PICK construct as suggested > by Matt Roper. > > v3: - DBUF_CTL_S* to _DBUF_CTL_S*, changed X to "slice" > in macro(Ville Syrjälä) > - Introduced enum for enumerating DBUF slices(Ville Syrjälä) > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 30 +++++++++++-------- > .../drm/i915/display/intel_display_power.h | 5 ++++ > drivers/gpu/drm/i915/i915_reg.h | 7 +++-- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 4 files changed, 28 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 5e1c601f0f99..a59efb24be92 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -4418,9 +4418,11 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, > return; > > if (req_slices > hw_enabled_slices) > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true); > + ret = intel_dbuf_slice_set(dev_priv, > + _DBUF_CTL_S(DBUF_S2), true); > else > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, false); > + ret = intel_dbuf_slice_set(dev_priv, > + _DBUF_CTL_S(DBUF_S2), false); > > if (ret) > dev_priv->enabled_dbuf_slices_num = req_slices; > @@ -4428,14 +4430,16 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, > > static void icl_dbuf_enable(struct drm_i915_private *dev_priv) > { > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | DBUF_POWER_REQUEST); > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) | DBUF_POWER_REQUEST); > - POSTING_READ(DBUF_CTL_S2); > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > + I915_READ(_DBUF_CTL_S(DBUF_S1)) | DBUF_POWER_REQUEST); > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > + I915_READ(_DBUF_CTL_S(DBUF_S2)) | DBUF_POWER_REQUEST); > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > udelay(10); > > - if (!(I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > - !(I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > + if (!(I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > + !(I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > DRM_ERROR("DBuf power enable timeout\n"); > else > /* > @@ -4447,14 +4451,16 @@ static void icl_dbuf_enable(struct drm_i915_private *dev_priv) > > static void icl_dbuf_disable(struct drm_i915_private *dev_priv) > { > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) & ~DBUF_POWER_REQUEST); > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) & ~DBUF_POWER_REQUEST); > - POSTING_READ(DBUF_CTL_S2); > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > + I915_READ(_DBUF_CTL_S(DBUF_S1)) & ~DBUF_POWER_REQUEST); > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > + I915_READ(_DBUF_CTL_S(DBUF_S2)) & ~DBUF_POWER_REQUEST); > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > udelay(10); > > - if ((I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > - (I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > + if ((I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > + (I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > DRM_ERROR("DBuf power disable timeout!\n"); > else > /* > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 2608a65af7fa..601e000ffd0d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -307,6 +307,11 @@ intel_display_power_put_async(struct drm_i915_private *i915, > } > #endif > > +enum dbuf_slice { > + DBUF_S1, > + DBUF_S2, > +}; > + > #define with_intel_display_power(i915, domain, wf) \ > for ((wf) = intel_display_power_get((i915), (domain)); (wf); \ > intel_display_power_put_async((i915), (domain), (wf)), (wf) = 0) > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index b93c4c18f05c..625be54d3eae 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7748,9 +7748,10 @@ enum { > #define DISP_ARB_CTL2 _MMIO(0x45004) > #define DISP_DATA_PARTITION_5_6 (1 << 6) > #define DISP_IPC_ENABLE (1 << 3) > -#define DBUF_CTL _MMIO(0x45008) > -#define DBUF_CTL_S1 _MMIO(0x45008) > -#define DBUF_CTL_S2 _MMIO(0x44FE8) > +#define DBUF_CTL_ADDR1 0x45008 > +#define DBUF_CTL_ADDR2 0x44FE8 > +#define _DBUF_CTL_S(X) _MMIO(_PICK_EVEN(X, DBUF_CTL_ADDR1, DBUF_CTL_ADDR2)) That's not at all what I meant. Also the 'X' is still there despite what the changelog says. #define _DBUF_CTL_S1 0x45008 #define _DBUF_CTL_S2 0x44FE8 #define DBUF_CTL_S(slice) _MMIO(_PICK_EVEN(slice, _DBUF_CTL_S1, _DBUF_CTL_S2)) > +#define DBUF_CTL _DBUF_CTL_S(0) > #define DBUF_POWER_REQUEST (1 << 31) > #define DBUF_POWER_STATE (1 << 30) > #define GEN7_MSG_CTL _MMIO(0x45010) > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 04f94057d6b3..b8d78e26515c 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3660,7 +3660,7 @@ u8 intel_enabled_dbuf_slices_num(struct drm_i915_private *dev_priv) > * only that 1 slice enabled until we have a proper way for on-demand > * toggling of the second slice. > */ > - if (0 && I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE) > + if (0 && I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE) > enabled_dbuf_slices_num++; > > return enabled_dbuf_slices_num; > -- > 2.24.1.485.gad05a3d8e5
On Tue, 2020-01-28 at 19:35 +0200, Ville Syrjälä wrote: > On Fri, Jan 24, 2020 at 10:44:52AM +0200, Stanislav Lisovskiy wrote: > > Now start using parameterized DBUF_CTL instead > > of hardcoded, this would allow shorter access > > functions when reading or storing entire state. > > > > Tried to implement it in a MMIO_PIPE manner, however > > DBUF_CTL1 address is higher than DBUF_CTL2, which > > implies that we have to now subtract from base > > rather than add. > > > > v2: - Removed unneeded DBUF_CTL_DIST and DBUF_CTL_ADDR > > macros. Started to use _PICK construct as suggested > > by Matt Roper. > > > > v3: - DBUF_CTL_S* to _DBUF_CTL_S*, changed X to "slice" > > in macro(Ville Syrjälä) > > - Introduced enum for enumerating DBUF slices(Ville Syrjälä) > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > .../drm/i915/display/intel_display_power.c | 30 +++++++++++-- > > ------ > > .../drm/i915/display/intel_display_power.h | 5 ++++ > > drivers/gpu/drm/i915/i915_reg.h | 7 +++-- > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > 4 files changed, 28 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 5e1c601f0f99..a59efb24be92 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -4418,9 +4418,11 @@ void icl_dbuf_slices_update(struct > > drm_i915_private *dev_priv, > > return; > > > > if (req_slices > hw_enabled_slices) > > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, > > true); > > + ret = intel_dbuf_slice_set(dev_priv, > > + _DBUF_CTL_S(DBUF_S2), true); > > else > > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, > > false); > > + ret = intel_dbuf_slice_set(dev_priv, > > + _DBUF_CTL_S(DBUF_S2), > > false); > > > > if (ret) > > dev_priv->enabled_dbuf_slices_num = req_slices; > > @@ -4428,14 +4430,16 @@ void icl_dbuf_slices_update(struct > > drm_i915_private *dev_priv, > > > > static void icl_dbuf_enable(struct drm_i915_private *dev_priv) > > { > > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | > > DBUF_POWER_REQUEST); > > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) | > > DBUF_POWER_REQUEST); > > - POSTING_READ(DBUF_CTL_S2); > > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > > + I915_READ(_DBUF_CTL_S(DBUF_S1)) | > > DBUF_POWER_REQUEST); > > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > > + I915_READ(_DBUF_CTL_S(DBUF_S2)) | > > DBUF_POWER_REQUEST); > > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > > > udelay(10); > > > > - if (!(I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > > - !(I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > > + if (!(I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > > + !(I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > > DRM_ERROR("DBuf power enable timeout\n"); > > else > > /* > > @@ -4447,14 +4451,16 @@ static void icl_dbuf_enable(struct > > drm_i915_private *dev_priv) > > > > static void icl_dbuf_disable(struct drm_i915_private *dev_priv) > > { > > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) & > > ~DBUF_POWER_REQUEST); > > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) & > > ~DBUF_POWER_REQUEST); > > - POSTING_READ(DBUF_CTL_S2); > > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > > + I915_READ(_DBUF_CTL_S(DBUF_S1)) & > > ~DBUF_POWER_REQUEST); > > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > > + I915_READ(_DBUF_CTL_S(DBUF_S2)) & > > ~DBUF_POWER_REQUEST); > > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > > > udelay(10); > > > > - if ((I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > > - (I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > > + if ((I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > > + (I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > > DRM_ERROR("DBuf power disable timeout!\n"); > > else > > /* > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > index 2608a65af7fa..601e000ffd0d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > @@ -307,6 +307,11 @@ intel_display_power_put_async(struct > > drm_i915_private *i915, > > } > > #endif > > > > +enum dbuf_slice { > > + DBUF_S1, > > + DBUF_S2, > > +}; > > + > > #define with_intel_display_power(i915, domain, wf) \ > > for ((wf) = intel_display_power_get((i915), (domain)); (wf); \ > > intel_display_power_put_async((i915), (domain), (wf)), > > (wf) = 0) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index b93c4c18f05c..625be54d3eae 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7748,9 +7748,10 @@ enum { > > #define DISP_ARB_CTL2 _MMIO(0x45004) > > #define DISP_DATA_PARTITION_5_6 (1 << 6) > > #define DISP_IPC_ENABLE (1 << 3) > > -#define DBUF_CTL _MMIO(0x45008) > > -#define DBUF_CTL_S1 _MMIO(0x45008) > > -#define DBUF_CTL_S2 _MMIO(0x44FE8) > > +#define DBUF_CTL_ADDR1 0x45008 > > +#define DBUF_CTL_ADDR2 0x44FE8 > > +#define _DBUF_CTL_S(X) _MMIO(_PICK_EVEN(X, > > DBUF_CTL_ADDR1, DBUF_CTL_ADDR2)) > > That's not at all what I meant. Also the 'X' is still there despite > what > the changelog says. > > #define _DBUF_CTL_S1 0x45008 > #define _DBUF_CTL_S2 0x44FE8 > #define DBUF_CTL_S(slice) _MMIO(_PICK_EVEN(slice, _DBUF_CTL_S1, > _DBUF_CTL_S2)) My idea was to still be able to use DBUF_CTL_S1 and DBUF_CTL_S2, of course this is a bit redundant, but though similar naming DBUF_CTL_S1 and DBUF_CTL_S(0) might confuse somebody into using it. For example in your case we can now only use DBUF_CTL_S() macro because _DBUF_CTL_S1/2 is not any longer using _MMIO. But _now_ I get your point, I guess by "_" in the beginning, it is meant not to be used from outside. Some kind of like private class members in Python :) I will change this once the rest of patches are reviewed, because that change anyway does not affect the actual functionality, but purely cosmetic. Stan > > > > +#define DBUF_CTL _DBUF_CTL_S(0) > > #define DBUF_POWER_REQUEST (1 << 31) > > #define DBUF_POWER_STATE (1 << 30) > > #define GEN7_MSG_CTL _MMIO(0x45010) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 04f94057d6b3..b8d78e26515c 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3660,7 +3660,7 @@ u8 intel_enabled_dbuf_slices_num(struct > > drm_i915_private *dev_priv) > > * only that 1 slice enabled until we have a proper way for on- > > demand > > * toggling of the second slice. > > */ > > - if (0 && I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE) > > + if (0 && I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE) > > enabled_dbuf_slices_num++; > > > > return enabled_dbuf_slices_num; > > -- > > 2.24.1.485.gad05a3d8e5 > >
On Wed, Jan 29, 2020 at 08:41:34AM +0000, Lisovskiy, Stanislav wrote: > On Tue, 2020-01-28 at 19:35 +0200, Ville Syrjälä wrote: > > On Fri, Jan 24, 2020 at 10:44:52AM +0200, Stanislav Lisovskiy wrote: > > > Now start using parameterized DBUF_CTL instead > > > of hardcoded, this would allow shorter access > > > functions when reading or storing entire state. > > > > > > Tried to implement it in a MMIO_PIPE manner, however > > > DBUF_CTL1 address is higher than DBUF_CTL2, which > > > implies that we have to now subtract from base > > > rather than add. > > > > > > v2: - Removed unneeded DBUF_CTL_DIST and DBUF_CTL_ADDR > > > macros. Started to use _PICK construct as suggested > > > by Matt Roper. > > > > > > v3: - DBUF_CTL_S* to _DBUF_CTL_S*, changed X to "slice" > > > in macro(Ville Syrjälä) > > > - Introduced enum for enumerating DBUF slices(Ville Syrjälä) > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > > --- > > > .../drm/i915/display/intel_display_power.c | 30 +++++++++++-- > > > ------ > > > .../drm/i915/display/intel_display_power.h | 5 ++++ > > > drivers/gpu/drm/i915/i915_reg.h | 7 +++-- > > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > > 4 files changed, 28 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > > index 5e1c601f0f99..a59efb24be92 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > > @@ -4418,9 +4418,11 @@ void icl_dbuf_slices_update(struct > > > drm_i915_private *dev_priv, > > > return; > > > > > > if (req_slices > hw_enabled_slices) > > > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, > > > true); > > > + ret = intel_dbuf_slice_set(dev_priv, > > > + _DBUF_CTL_S(DBUF_S2), true); > > > else > > > - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, > > > false); > > > + ret = intel_dbuf_slice_set(dev_priv, > > > + _DBUF_CTL_S(DBUF_S2), > > > false); > > > > > > if (ret) > > > dev_priv->enabled_dbuf_slices_num = req_slices; > > > @@ -4428,14 +4430,16 @@ void icl_dbuf_slices_update(struct > > > drm_i915_private *dev_priv, > > > > > > static void icl_dbuf_enable(struct drm_i915_private *dev_priv) > > > { > > > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | > > > DBUF_POWER_REQUEST); > > > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) | > > > DBUF_POWER_REQUEST); > > > - POSTING_READ(DBUF_CTL_S2); > > > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > > > + I915_READ(_DBUF_CTL_S(DBUF_S1)) | > > > DBUF_POWER_REQUEST); > > > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > > > + I915_READ(_DBUF_CTL_S(DBUF_S2)) | > > > DBUF_POWER_REQUEST); > > > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > > > > > udelay(10); > > > > > > - if (!(I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > > > - !(I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > > > + if (!(I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > > > + !(I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > > > DRM_ERROR("DBuf power enable timeout\n"); > > > else > > > /* > > > @@ -4447,14 +4451,16 @@ static void icl_dbuf_enable(struct > > > drm_i915_private *dev_priv) > > > > > > static void icl_dbuf_disable(struct drm_i915_private *dev_priv) > > > { > > > - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) & > > > ~DBUF_POWER_REQUEST); > > > - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) & > > > ~DBUF_POWER_REQUEST); > > > - POSTING_READ(DBUF_CTL_S2); > > > + I915_WRITE(_DBUF_CTL_S(DBUF_S1), > > > + I915_READ(_DBUF_CTL_S(DBUF_S1)) & > > > ~DBUF_POWER_REQUEST); > > > + I915_WRITE(_DBUF_CTL_S(DBUF_S2), > > > + I915_READ(_DBUF_CTL_S(DBUF_S2)) & > > > ~DBUF_POWER_REQUEST); > > > + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); > > > > > > udelay(10); > > > > > > - if ((I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || > > > - (I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) > > > + if ((I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || > > > + (I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) > > > DRM_ERROR("DBuf power disable timeout!\n"); > > > else > > > /* > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > > index 2608a65af7fa..601e000ffd0d 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > > @@ -307,6 +307,11 @@ intel_display_power_put_async(struct > > > drm_i915_private *i915, > > > } > > > #endif > > > > > > +enum dbuf_slice { > > > + DBUF_S1, > > > + DBUF_S2, > > > +}; > > > + > > > #define with_intel_display_power(i915, domain, wf) \ > > > for ((wf) = intel_display_power_get((i915), (domain)); (wf); \ > > > intel_display_power_put_async((i915), (domain), (wf)), > > > (wf) = 0) > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index b93c4c18f05c..625be54d3eae 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -7748,9 +7748,10 @@ enum { > > > #define DISP_ARB_CTL2 _MMIO(0x45004) > > > #define DISP_DATA_PARTITION_5_6 (1 << 6) > > > #define DISP_IPC_ENABLE (1 << 3) > > > -#define DBUF_CTL _MMIO(0x45008) > > > -#define DBUF_CTL_S1 _MMIO(0x45008) > > > -#define DBUF_CTL_S2 _MMIO(0x44FE8) > > > +#define DBUF_CTL_ADDR1 0x45008 > > > +#define DBUF_CTL_ADDR2 0x44FE8 > > > +#define _DBUF_CTL_S(X) _MMIO(_PICK_EVEN(X, > > > DBUF_CTL_ADDR1, DBUF_CTL_ADDR2)) > > > > That's not at all what I meant. Also the 'X' is still there despite > > what > > the changelog says. > > > > #define _DBUF_CTL_S1 0x45008 > > #define _DBUF_CTL_S2 0x44FE8 > > #define DBUF_CTL_S(slice) _MMIO(_PICK_EVEN(slice, _DBUF_CTL_S1, > > _DBUF_CTL_S2)) > > My idea was to still be able to use DBUF_CTL_S1 and DBUF_CTL_S2, of Just don't. Aliases are confusing. > course this is a bit redundant, but though similar naming DBUF_CTL_S1 > and DBUF_CTL_S(0) might confuse somebody into using it. > For example in your case we can now only use DBUF_CTL_S() macro because > _DBUF_CTL_S1/2 is not any longer using _MMIO. > > But _now_ I get your point, I guess by "_" in the beginning, it is > meant not to be used from outside. Some kind of like private class > members in Python :) > > I will change this once the rest of patches are reviewed, because that > change anyway does not affect the actual functionality, but purely > cosmetic. > > Stan > > > > > > > > +#define DBUF_CTL _DBUF_CTL_S(0) > > > #define DBUF_POWER_REQUEST (1 << 31) > > > #define DBUF_POWER_STATE (1 << 30) > > > #define GEN7_MSG_CTL _MMIO(0x45010) > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 04f94057d6b3..b8d78e26515c 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -3660,7 +3660,7 @@ u8 intel_enabled_dbuf_slices_num(struct > > > drm_i915_private *dev_priv) > > > * only that 1 slice enabled until we have a proper way for on- > > > demand > > > * toggling of the second slice. > > > */ > > > - if (0 && I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE) > > > + if (0 && I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE) > > > enabled_dbuf_slices_num++; > > > > > > return enabled_dbuf_slices_num; > > > -- > > > 2.24.1.485.gad05a3d8e5 > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 5e1c601f0f99..a59efb24be92 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -4418,9 +4418,11 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, return; if (req_slices > hw_enabled_slices) - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true); + ret = intel_dbuf_slice_set(dev_priv, + _DBUF_CTL_S(DBUF_S2), true); else - ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, false); + ret = intel_dbuf_slice_set(dev_priv, + _DBUF_CTL_S(DBUF_S2), false); if (ret) dev_priv->enabled_dbuf_slices_num = req_slices; @@ -4428,14 +4430,16 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, static void icl_dbuf_enable(struct drm_i915_private *dev_priv) { - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | DBUF_POWER_REQUEST); - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) | DBUF_POWER_REQUEST); - POSTING_READ(DBUF_CTL_S2); + I915_WRITE(_DBUF_CTL_S(DBUF_S1), + I915_READ(_DBUF_CTL_S(DBUF_S1)) | DBUF_POWER_REQUEST); + I915_WRITE(_DBUF_CTL_S(DBUF_S2), + I915_READ(_DBUF_CTL_S(DBUF_S2)) | DBUF_POWER_REQUEST); + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); udelay(10); - if (!(I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || - !(I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) + if (!(I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || + !(I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) DRM_ERROR("DBuf power enable timeout\n"); else /* @@ -4447,14 +4451,16 @@ static void icl_dbuf_enable(struct drm_i915_private *dev_priv) static void icl_dbuf_disable(struct drm_i915_private *dev_priv) { - I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) & ~DBUF_POWER_REQUEST); - I915_WRITE(DBUF_CTL_S2, I915_READ(DBUF_CTL_S2) & ~DBUF_POWER_REQUEST); - POSTING_READ(DBUF_CTL_S2); + I915_WRITE(_DBUF_CTL_S(DBUF_S1), + I915_READ(_DBUF_CTL_S(DBUF_S1)) & ~DBUF_POWER_REQUEST); + I915_WRITE(_DBUF_CTL_S(DBUF_S2), + I915_READ(_DBUF_CTL_S(DBUF_S2)) & ~DBUF_POWER_REQUEST); + POSTING_READ(_DBUF_CTL_S(DBUF_S2)); udelay(10); - if ((I915_READ(DBUF_CTL_S1) & DBUF_POWER_STATE) || - (I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE)) + if ((I915_READ(_DBUF_CTL_S(DBUF_S1)) & DBUF_POWER_STATE) || + (I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE)) DRM_ERROR("DBuf power disable timeout!\n"); else /* diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index 2608a65af7fa..601e000ffd0d 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -307,6 +307,11 @@ intel_display_power_put_async(struct drm_i915_private *i915, } #endif +enum dbuf_slice { + DBUF_S1, + DBUF_S2, +}; + #define with_intel_display_power(i915, domain, wf) \ for ((wf) = intel_display_power_get((i915), (domain)); (wf); \ intel_display_power_put_async((i915), (domain), (wf)), (wf) = 0) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b93c4c18f05c..625be54d3eae 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7748,9 +7748,10 @@ enum { #define DISP_ARB_CTL2 _MMIO(0x45004) #define DISP_DATA_PARTITION_5_6 (1 << 6) #define DISP_IPC_ENABLE (1 << 3) -#define DBUF_CTL _MMIO(0x45008) -#define DBUF_CTL_S1 _MMIO(0x45008) -#define DBUF_CTL_S2 _MMIO(0x44FE8) +#define DBUF_CTL_ADDR1 0x45008 +#define DBUF_CTL_ADDR2 0x44FE8 +#define _DBUF_CTL_S(X) _MMIO(_PICK_EVEN(X, DBUF_CTL_ADDR1, DBUF_CTL_ADDR2)) +#define DBUF_CTL _DBUF_CTL_S(0) #define DBUF_POWER_REQUEST (1 << 31) #define DBUF_POWER_STATE (1 << 30) #define GEN7_MSG_CTL _MMIO(0x45010) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 04f94057d6b3..b8d78e26515c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3660,7 +3660,7 @@ u8 intel_enabled_dbuf_slices_num(struct drm_i915_private *dev_priv) * only that 1 slice enabled until we have a proper way for on-demand * toggling of the second slice. */ - if (0 && I915_READ(DBUF_CTL_S2) & DBUF_POWER_STATE) + if (0 && I915_READ(_DBUF_CTL_S(DBUF_S2)) & DBUF_POWER_STATE) enabled_dbuf_slices_num++; return enabled_dbuf_slices_num;
Now start using parameterized DBUF_CTL instead of hardcoded, this would allow shorter access functions when reading or storing entire state. Tried to implement it in a MMIO_PIPE manner, however DBUF_CTL1 address is higher than DBUF_CTL2, which implies that we have to now subtract from base rather than add. v2: - Removed unneeded DBUF_CTL_DIST and DBUF_CTL_ADDR macros. Started to use _PICK construct as suggested by Matt Roper. v3: - DBUF_CTL_S* to _DBUF_CTL_S*, changed X to "slice" in macro(Ville Syrjälä) - Introduced enum for enumerating DBUF slices(Ville Syrjälä) Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> --- .../drm/i915/display/intel_display_power.c | 30 +++++++++++-------- .../drm/i915/display/intel_display_power.h | 5 ++++ drivers/gpu/drm/i915/i915_reg.h | 7 +++-- drivers/gpu/drm/i915/intel_pm.c | 2 +- 4 files changed, 28 insertions(+), 16 deletions(-)