> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Thursday, April 14, 2022 12:56 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915: Eliminate PIPECONF RMWs from
> .color_commit()
>
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> Eliminate the PIPECONF RMWs from .comit_commit() so that we can finally
> declare
> the whole vblank evade part (and the noarm() part) of the pipe commit free of
> register reads. Or at least I hope that's the last read...
>
> Only the i9xx/ilk codepaths need this for now, but let's add the same thing
> for hsw+
> just in case we want to start calling that during fastsets at some point (eg.
> to change
> dithering settings/etc.).
>
> Should open up the way to start experimenting with different DSB usage
> approaches
> for pipe commits.
Looks Good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 21 ++++----------
> drivers/gpu/drm/i915/display/intel_display.c | 30 ++++++++++++++------
> drivers/gpu/drm/i915/display/intel_display.h | 2 ++
> 3 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 34128c9c635c..60532dd0f9f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -505,30 +505,19 @@ static void ilk_color_commit_noarm(const struct
> intel_crtc_state *crtc_state)
>
> static void i9xx_color_commit_arm(const struct intel_crtc_state *crtc_state)
> {
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum pipe pipe = crtc->pipe;
> - u32 val;
> -
> - val = intel_de_read(dev_priv, PIPECONF(pipe));
> - val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> - val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> - intel_de_write(dev_priv, PIPECONF(pipe), val);
> + /* update PIPECONF GAMMA_MODE */
> + i9xx_set_pipeconf(crtc_state);
> }
>
> static void ilk_color_commit_arm(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum pipe pipe = crtc->pipe;
> - u32 val;
>
> - val = intel_de_read(dev_priv, PIPECONF(pipe));
> - val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> - val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> - intel_de_write(dev_priv, PIPECONF(pipe), val);
> + /* update PIPECONF GAMMA_MODE */
> + ilk_set_pipeconf(crtc_state);
>
> - intel_de_write_fw(dev_priv, PIPE_CSC_MODE(pipe),
> + intel_de_write_fw(dev_priv, PIPE_CSC_MODE(crtc->pipe),
> crtc_state->csc_mode);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 29044cf58b87..aa2814332ad9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -122,8 +122,6 @@
>
> static void intel_set_transcoder_timings(const struct intel_crtc_state
> *crtc_state);
> static void intel_set_pipe_src_size(const struct intel_crtc_state
> *crtc_state); -static
> void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state); -static
> void
> ilk_set_pipeconf(const struct intel_crtc_state *crtc_state); static void
> hsw_set_transconf(const struct intel_crtc_state *crtc_state); static void
> bdw_set_pipemisc(const struct intel_crtc_state *crtc_state); static void
> ilk_pfit_enable(const struct intel_crtc_state *crtc_state); @@ -3205,14
> +3203,18
> @@ static void intel_get_pipe_src_size(struct intel_crtc *crtc,
> intel_bigjoiner_adjust_pipe_src(pipe_config);
> }
>
> -static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> +void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 pipeconf = 0;
>
> - /* we keep both pipes enabled on 830 */
> - if (IS_I830(dev_priv))
> + /*
> + * - We keep both pipes enabled on 830
> + * - During modeset the pipe is still disabled and must remain so
> + * - During fastset the pipe is already enabled and must remain so
> + */
> + if (IS_I830(dev_priv) || !intel_crtc_needs_modeset(crtc_state))
> pipeconf |= PIPECONF_ENABLE;
>
> if (crtc_state->double_wide)
> @@ -3524,14 +3526,19 @@ static bool i9xx_get_pipe_config(struct intel_crtc
> *crtc,
> return ret;
> }
>
> -static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
> +void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum pipe pipe = crtc->pipe;
> - u32 val;
> + u32 val = 0;
>
> - val = 0;
> + /*
> + * - During modeset the pipe is still disabled and must remain so
> + * - During fastset the pipe is already enabled and must remain so
> + */
> + if (!intel_crtc_needs_modeset(crtc_state))
> + val |= PIPECONF_ENABLE;
>
> switch (crtc_state->pipe_bpp) {
> case 18:
> @@ -3589,6 +3596,13 @@ static void hsw_set_transconf(const struct
> intel_crtc_state *crtc_state)
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> u32 val = 0;
>
> + /*
> + * - During modeset the pipe is still disabled and must remain so
> + * - During fastset the pipe is already enabled and must remain so
> + */
> + if (!intel_crtc_needs_modeset(crtc_state))
> + val |= PIPECONF_ENABLE;
> +
> if (IS_HASWELL(dev_priv) && crtc_state->dither)
> val |= PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 867fa248f042..ee488205c5fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -561,6 +561,8 @@ u8 intel_crtc_bigjoiner_slave_pipes(const struct
> intel_crtc_state *crtc_state); struct intel_crtc *intel_master_crtc(const
> struct
> intel_crtc_state *crtc_state);
>
> void intel_plane_destroy(struct drm_plane *plane);
> +void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state); void
> +ilk_set_pipeconf(const struct intel_crtc_state *crtc_state);
> void intel_enable_transcoder(const struct intel_crtc_state *new_crtc_state);
> void
> intel_disable_transcoder(const struct intel_crtc_state *old_crtc_state); void
> i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> --
> 2.35.1
