| Message ID | 20220413192607.27533-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Eliminate PIPECONF RMWs from .color_commit() | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@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.syrjala@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.shankar@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@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
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);