[PATCH 02/18] drm/i915: Move linetime wms into the crtc state

[Ville Syrjälä]

On Wed, Jan 29, 2020 at 02:03:47PM +0000, Lisovskiy, Stanislav wrote:
> On Fri, 2020-01-17 at 23:23 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > The linetime watermarks really have very little in common with the
> > plane watermarks. It looks to be cleaner to simply track them in
> > the crtc_state and program them from the normal modeset/fastset
> > paths.
> > 
> > The only dark cloud comes from the fact that the register is
> > still supposedly single buffered. So in theory it might still
> > need some form of two stage programming. Note that even though
> > HSW/BDWhave two stage programming we never computed any special
> > intermediate values for the linetime watermarks, and on SKL+
> > we don't even have the two stage stuff plugged in since everything
> > else is double buffered. So let's assume it's all fine and
> > continue doing what we've been doing.
> > 
> > Actually on HSW/BDW the value should not even change without
> > a full modeset since it doesn't account for pfit downscaling.
> > Thus only fastboot might be affected. But on SKL+ the pfit
> > scaling factor is take into consideration so the value may
> > change during any fastset.
> > 
> > As a bonus we'll plug this thing into the state
> > checker/dump now.
> > 
> > v2: Rebase due to bigjoiner prep
> > v2: Only compute ips linetime for IPS capable pipes.
> >     Bspec says the register values is ignored for other
> >     pipes, but in fact it can't even be written so the
> >     state checker becomes unhappy if we don't compute
> >     it as zero.
> > 
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  95 +++++++++++++-
> >  .../drm/i915/display/intel_display_types.h    |   7 +-
> >  drivers/gpu/drm/i915/i915_drv.h               |   1 -
> >  drivers/gpu/drm/i915/intel_pm.c               | 117 +---------------
> > --
> >  4 files changed, 98 insertions(+), 122 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index dd03987cc24f..037898c2731a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6885,6 +6885,16 @@ static void icl_pipe_mbus_enable(struct
> > intel_crtc *crtc)
> >  	I915_WRITE(PIPE_MBUS_DBOX_CTL(pipe), val);
> >  }
> >  
> > +static void hsw_set_linetime_wm(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);
> > +
> > +	I915_WRITE(WM_LINETIME(crtc->pipe),
> > +		   HSW_LINETIME(crtc_state->linetime) |
> > +		   HSW_IPS_LINETIME(crtc_state->ips_linetime));
> > +}
> > +
> >  static void hsw_set_frame_start_delay(const struct intel_crtc_state
> > *crtc_state)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -6969,6 +6979,8 @@ static void hsw_crtc_enable(struct
> > intel_atomic_state *state,
> >  	if (INTEL_GEN(dev_priv) < 9)
> >  		intel_disable_primary_plane(new_crtc_state);
> >  
> > +	hsw_set_linetime_wm(new_crtc_state);
> > +
> >  	if (INTEL_GEN(dev_priv) >= 11)
> >  		icl_set_pipe_chicken(crtc);
> >  
> > @@ -10947,6 +10959,7 @@ static bool hsw_get_pipe_config(struct
> > intel_crtc *crtc,
> >  	enum intel_display_power_domain power_domain;
> >  	u64 power_domain_mask;
> >  	bool active;
> > +	u32 tmp;
> >  
> >  	pipe_config->master_transcoder = INVALID_TRANSCODER;
> >  
> > @@ -11010,7 +11023,7 @@ static bool hsw_get_pipe_config(struct
> > intel_crtc *crtc,
> >  	pipe_config->csc_mode = I915_READ(PIPE_CSC_MODE(crtc->pipe));
> >  
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> > -		u32 tmp = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
> > +		tmp = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
> >  
> >  		if (tmp & SKL_BOTTOM_COLOR_GAMMA_ENABLE)
> >  			pipe_config->gamma_enable = true;
> > @@ -11023,6 +11036,12 @@ static bool hsw_get_pipe_config(struct
> > intel_crtc *crtc,
> >  
> >  	intel_color_get_config(pipe_config);
> >  
> > +	tmp = I915_READ(WM_LINETIME(crtc->pipe));
> > +	pipe_config->linetime = REG_FIELD_GET(HSW_LINETIME_MASK, tmp);
> 
> Shouldn't we have also gen >= 9 check here? If let's say
> hsw_get_pipe_config was called for non-gen 9, non-hsw and non-bdw case.

It is not. Its name is hsw_whatever for a reason.

> 
> I see that hsw_get_pipe_config hook is used also for any platform which
> evaluates HAS_DDI check to true in intel_init_display_hooks.

Which is another way to say HSW+

> 
> At least I see that everywhere else there is a check for >= gen 9,
> when you set pipe_config->linetime. 

HSW/BDW already have limetime wm. For SKL+ we just compute it a bit
differently, hence sometimes we need a gen>=9 check.

-- 
Ville Syrjälä
Intel