| Message ID | 20220421192205.32649-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm: Add infrastructure to allow seamless mode switches | expand |
On Thu, 21 Apr 2022, José Roberto de Souza <jose.souza@intel.com> wrote: > Will be adding some additional control options to DRRS that will > require to have the DRRS downclock mode stored in the crtc_state. > > So to optimize memory usage a bit here using it to replace has_drrs > as we can check if the drrs_downclock_mode pointer is different than > null to have the same behavior has has_drrs. > > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 4 ++-- > drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++- > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > drivers/gpu/drm/i915/display/intel_drrs.c | 4 ++-- > 5 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 0ddfce21a828d..a5f5caeced9a0 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5360,7 +5360,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, > > drm_dbg_kms(&dev_priv->drm, "ips: %i, double wide: %i, drrs: %i\n", > pipe_config->ips_enabled, pipe_config->double_wide, > - pipe_config->has_drrs); > + CRTC_STATE_HAS_DRRS(pipe_config)); I'll mostly let Ville comment on the series, but that macro is an eyesore and also just out of place in intel_display_types.h. Please make it a proper function intel_drrs_something_something() in intel_drrs.[ch]. BR, Jani. > > intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state); > > @@ -7088,7 +7088,7 @@ static void intel_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_stat > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > - new_crtc_state->has_drrs = old_crtc_state->has_drrs; > + new_crtc_state->drrs_downclock_mode = old_crtc_state->drrs_downclock_mode; > } > > static int intel_crtc_add_planes_to_state(struct intel_atomic_state *state, > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 452d773fd4e34..f9720562336da 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -1096,7 +1096,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused) > > /* DRRS Supported */ > seq_printf(m, "\tDRRS Enabled: %s\n", > - str_yes_no(crtc_state->has_drrs)); > + str_yes_no(CRTC_STATE_HAS_DRRS(crtc_state))); > > seq_printf(m, "\tDRRS Active: %s\n", > str_yes_no(intel_drrs_is_active(crtc))); > @@ -1786,7 +1786,7 @@ static int i915_drrs_ctl_set(void *data, u64 val) > crtc_state = to_intel_crtc_state(crtc->base.state); > > if (!crtc_state->hw.active || > - !crtc_state->has_drrs) > + !CRTC_STATE_HAS_DRRS(crtc_state)) > goto out; > > commit = crtc_state->uapi.commit; > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index cfd042117b109..f0b3cfd3138ce 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1056,7 +1056,7 @@ struct intel_crtc_state { > > /* m2_n2 for eDP downclock */ > struct intel_link_m_n dp_m2_n2; > - bool has_drrs; > + const struct drm_display_mode *drrs_downclock_mode; > > /* PSR is supported but might not be enabled due the lack of enabled planes */ > bool has_psr; > @@ -1264,6 +1264,8 @@ enum drrs_refresh_rate { > DRRS_REFRESH_RATE_LOW, > }; > > +#define CRTC_STATE_HAS_DRRS(crtc_state) (!!((crtc_state)->drrs_downclock_mode)) > + > #define INTEL_PIPE_CRC_ENTRIES_NR 128 > struct intel_pipe_crc { > spinlock_t lock; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index d55acc4a028a8..feea172dd2753 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -1881,7 +1881,7 @@ intel_dp_drrs_compute_config(struct intel_connector *connector, > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || IS_IVYBRIDGE(i915)) > pipe_config->msa_timing_delay = i915->vbt.edp.drrs_msa_timing_delay; > > - pipe_config->has_drrs = true; > + pipe_config->drrs_downclock_mode = downclock_mode; > > pixel_clock = downclock_mode->clock; > if (pipe_config->splitter.enable) > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c > index 166caf293f7bc..dd527dfc2d1d5 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > @@ -144,7 +144,7 @@ void intel_drrs_activate(const struct intel_crtc_state *crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > - if (!crtc_state->has_drrs) > + if (!CRTC_STATE_HAS_DRRS(crtc_state)) > return; > > if (!crtc_state->hw.active) > @@ -176,7 +176,7 @@ void intel_drrs_deactivate(const struct intel_crtc_state *old_crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > > - if (!old_crtc_state->has_drrs) > + if (!CRTC_STATE_HAS_DRRS(old_crtc_state)) > return; > > if (!old_crtc_state->hw.active)
On Mon, 2022-04-25 at 14:55 +0300, Jani Nikula wrote: > On Thu, 21 Apr 2022, José Roberto de Souza <jose.souza@intel.com> wrote: > > Will be adding some additional control options to DRRS that will > > require to have the DRRS downclock mode stored in the crtc_state. > > > > So to optimize memory usage a bit here using it to replace has_drrs > > as we can check if the drrs_downclock_mode pointer is different than > > null to have the same behavior has has_drrs. > > > > Cc: Vidya Srinivas <vidya.srinivas@intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 4 ++-- > > drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++- > > drivers/gpu/drm/i915/display/intel_dp.c | 2 +- > > drivers/gpu/drm/i915/display/intel_drrs.c | 4 ++-- > > 5 files changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 0ddfce21a828d..a5f5caeced9a0 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5360,7 +5360,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, > > > > drm_dbg_kms(&dev_priv->drm, "ips: %i, double wide: %i, drrs: %i\n", > > pipe_config->ips_enabled, pipe_config->double_wide, > > - pipe_config->has_drrs); > > + CRTC_STATE_HAS_DRRS(pipe_config)); > > I'll mostly let Ville comment on the series, but that macro is an > eyesore and also just out of place in intel_display_types.h. Please make > it a proper function intel_drrs_something_something() in > intel_drrs.[ch]. Okay in making this a function but I don't have a good name for it neither. intel_crtc_state_has_drrs() is worst than current name of the macro. > > BR, > Jani. > > > > > intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state); > > > > @@ -7088,7 +7088,7 @@ static void intel_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_stat > > new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; > > new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; > > new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; > > - new_crtc_state->has_drrs = old_crtc_state->has_drrs; > > + new_crtc_state->drrs_downclock_mode = old_crtc_state->drrs_downclock_mode; > > } > > > > static int intel_crtc_add_planes_to_state(struct intel_atomic_state *state, > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 452d773fd4e34..f9720562336da 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -1096,7 +1096,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused) > > > > /* DRRS Supported */ > > seq_printf(m, "\tDRRS Enabled: %s\n", > > - str_yes_no(crtc_state->has_drrs)); > > + str_yes_no(CRTC_STATE_HAS_DRRS(crtc_state))); > > > > seq_printf(m, "\tDRRS Active: %s\n", > > str_yes_no(intel_drrs_is_active(crtc))); > > @@ -1786,7 +1786,7 @@ static int i915_drrs_ctl_set(void *data, u64 val) > > crtc_state = to_intel_crtc_state(crtc->base.state); > > > > if (!crtc_state->hw.active || > > - !crtc_state->has_drrs) > > + !CRTC_STATE_HAS_DRRS(crtc_state)) > > goto out; > > > > commit = crtc_state->uapi.commit; > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > > index cfd042117b109..f0b3cfd3138ce 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1056,7 +1056,7 @@ struct intel_crtc_state { > > > > /* m2_n2 for eDP downclock */ > > struct intel_link_m_n dp_m2_n2; > > - bool has_drrs; > > + const struct drm_display_mode *drrs_downclock_mode; > > > > /* PSR is supported but might not be enabled due the lack of enabled planes */ > > bool has_psr; > > @@ -1264,6 +1264,8 @@ enum drrs_refresh_rate { > > DRRS_REFRESH_RATE_LOW, > > }; > > > > +#define CRTC_STATE_HAS_DRRS(crtc_state) (!!((crtc_state)->drrs_downclock_mode)) > > + > > #define INTEL_PIPE_CRC_ENTRIES_NR 128 > > struct intel_pipe_crc { > > spinlock_t lock; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index d55acc4a028a8..feea172dd2753 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -1881,7 +1881,7 @@ intel_dp_drrs_compute_config(struct intel_connector *connector, > > if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || IS_IVYBRIDGE(i915)) > > pipe_config->msa_timing_delay = i915->vbt.edp.drrs_msa_timing_delay; > > > > - pipe_config->has_drrs = true; > > + pipe_config->drrs_downclock_mode = downclock_mode; > > > > pixel_clock = downclock_mode->clock; > > if (pipe_config->splitter.enable) > > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c > > index 166caf293f7bc..dd527dfc2d1d5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > > @@ -144,7 +144,7 @@ void intel_drrs_activate(const struct intel_crtc_state *crtc_state) > > { > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > > - if (!crtc_state->has_drrs) > > + if (!CRTC_STATE_HAS_DRRS(crtc_state)) > > return; > > > > if (!crtc_state->hw.active) > > @@ -176,7 +176,7 @@ void intel_drrs_deactivate(const struct intel_crtc_state *old_crtc_state) > > { > > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); > > > > - if (!old_crtc_state->has_drrs) > > + if (!CRTC_STATE_HAS_DRRS(old_crtc_state)) > > return; > > > > if (!old_crtc_state->hw.active) >
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 0ddfce21a828d..a5f5caeced9a0 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5360,7 +5360,7 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, drm_dbg_kms(&dev_priv->drm, "ips: %i, double wide: %i, drrs: %i\n", pipe_config->ips_enabled, pipe_config->double_wide, - pipe_config->has_drrs); + CRTC_STATE_HAS_DRRS(pipe_config)); intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state); @@ -7088,7 +7088,7 @@ static void intel_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_stat new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n; new_crtc_state->dp_m_n = old_crtc_state->dp_m_n; new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2; - new_crtc_state->has_drrs = old_crtc_state->has_drrs; + new_crtc_state->drrs_downclock_mode = old_crtc_state->drrs_downclock_mode; } static int intel_crtc_add_planes_to_state(struct intel_atomic_state *state, diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 452d773fd4e34..f9720562336da 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -1096,7 +1096,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused) /* DRRS Supported */ seq_printf(m, "\tDRRS Enabled: %s\n", - str_yes_no(crtc_state->has_drrs)); + str_yes_no(CRTC_STATE_HAS_DRRS(crtc_state))); seq_printf(m, "\tDRRS Active: %s\n", str_yes_no(intel_drrs_is_active(crtc))); @@ -1786,7 +1786,7 @@ static int i915_drrs_ctl_set(void *data, u64 val) crtc_state = to_intel_crtc_state(crtc->base.state); if (!crtc_state->hw.active || - !crtc_state->has_drrs) + !CRTC_STATE_HAS_DRRS(crtc_state)) goto out; commit = crtc_state->uapi.commit; diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index cfd042117b109..f0b3cfd3138ce 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1056,7 +1056,7 @@ struct intel_crtc_state { /* m2_n2 for eDP downclock */ struct intel_link_m_n dp_m2_n2; - bool has_drrs; + const struct drm_display_mode *drrs_downclock_mode; /* PSR is supported but might not be enabled due the lack of enabled planes */ bool has_psr; @@ -1264,6 +1264,8 @@ enum drrs_refresh_rate { DRRS_REFRESH_RATE_LOW, }; +#define CRTC_STATE_HAS_DRRS(crtc_state) (!!((crtc_state)->drrs_downclock_mode)) + #define INTEL_PIPE_CRC_ENTRIES_NR 128 struct intel_pipe_crc { spinlock_t lock; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index d55acc4a028a8..feea172dd2753 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1881,7 +1881,7 @@ intel_dp_drrs_compute_config(struct intel_connector *connector, if (IS_IRONLAKE(i915) || IS_SANDYBRIDGE(i915) || IS_IVYBRIDGE(i915)) pipe_config->msa_timing_delay = i915->vbt.edp.drrs_msa_timing_delay; - pipe_config->has_drrs = true; + pipe_config->drrs_downclock_mode = downclock_mode; pixel_clock = downclock_mode->clock; if (pipe_config->splitter.enable) diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c index 166caf293f7bc..dd527dfc2d1d5 100644 --- a/drivers/gpu/drm/i915/display/intel_drrs.c +++ b/drivers/gpu/drm/i915/display/intel_drrs.c @@ -144,7 +144,7 @@ void intel_drrs_activate(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); - if (!crtc_state->has_drrs) + if (!CRTC_STATE_HAS_DRRS(crtc_state)) return; if (!crtc_state->hw.active) @@ -176,7 +176,7 @@ void intel_drrs_deactivate(const struct intel_crtc_state *old_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); - if (!old_crtc_state->has_drrs) + if (!CRTC_STATE_HAS_DRRS(old_crtc_state)) return; if (!old_crtc_state->hw.active)
Will be adding some additional control options to DRRS that will require to have the DRRS downclock mode stored in the crtc_state. So to optimize memory usage a bit here using it to replace has_drrs as we can check if the drrs_downclock_mode pointer is different than null to have the same behavior has has_drrs. Cc: Vidya Srinivas <vidya.srinivas@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- drivers/gpu/drm/i915/display/intel_display_debugfs.c | 4 ++-- drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_drrs.c | 4 ++-- 5 files changed, 10 insertions(+), 8 deletions(-)