On Mon, 2022-04-25 at 14:55 +0300, Jani Nikula wrote:
> On Thu, 21 Apr 2022, José Roberto de Souza <jose.so...@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.srini...@intel.com>
> > Cc: Sean Paul <seanp...@chromium.org>
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.so...@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)
>
