diff mbox series

[v2,13/17] drm/i915: Stop using mode->private_flags

Message ID 20200403204008.14864-14-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm: Put drm_display_mode on diet | expand

Commit Message

Ville Syrjala April 3, 2020, 8:40 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the use of mode->private_flags with a truly private bitmaks
in our own crtc state. We also need a copy in the crtc itself so the
vblank code can get at it. We already have scanline_offset in there
for a similar reason, as well as the vblank->hwmode which is assigned
via drm_calc_timestamping_constants(). Fortunately we now have a
nice place for doing the crtc_state->crtc copy in
intel_crtc_update_active_timings() which gets called both for
modesets and init/resume readout.

The one slightly iffy spot is the INHERITED flag which we want to
preserve until userspace/fb_helper does the first proper commit after
actually calling .detecti() on the connectors. Otherwise we don't have
the full sink capabilities (audio,infoframes,etc.) when .compute_config()
gets called and thus we will fail to enable those features when the
first userspace commit happens. The only internal commit we do prior to
that should be from intel_initial_commit() and there we can simply
preserve the INHERITED flag from the readout.

CC: Sam Ravnborg <sam@ravnborg.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c        | 13 ++++------
 drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
 drivers/gpu/drm/i915/display/intel_display.c  | 24 +++++++++++++------
 .../drm/i915/display/intel_display_types.h    |  9 ++++++-
 drivers/gpu/drm/i915/display/intel_tv.c       |  4 ++--
 drivers/gpu/drm/i915/display/vlv_dsi.c        |  6 ++---
 drivers/gpu/drm/i915/i915_irq.c               |  4 ++--
 7 files changed, 37 insertions(+), 24 deletions(-)

Comments

Daniel Vetter April 7, 2020, 7:38 a.m. UTC | #1
On Fri, Apr 03, 2020 at 11:40:04PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the use of mode->private_flags with a truly private bitmaks
> in our own crtc state. We also need a copy in the crtc itself so the
> vblank code can get at it. We already have scanline_offset in there
> for a similar reason, as well as the vblank->hwmode which is assigned
> via drm_calc_timestamping_constants(). Fortunately we now have a
> nice place for doing the crtc_state->crtc copy in
> intel_crtc_update_active_timings() which gets called both for
> modesets and init/resume readout.
> 
> The one slightly iffy spot is the INHERITED flag which we want to
> preserve until userspace/fb_helper does the first proper commit after
> actually calling .detecti() on the connectors. Otherwise we don't have
> the full sink capabilities (audio,infoframes,etc.) when .compute_config()
> gets called and thus we will fail to enable those features when the
> first userspace commit happens. The only internal commit we do prior to
> that should be from intel_initial_commit() and there we can simply
> preserve the INHERITED flag from the readout.
> 
> CC: Sam Ravnborg <sam@ravnborg.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Emil Velikov <emil.l.velikov@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 13 ++++------
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
>  drivers/gpu/drm/i915/display/intel_display.c  | 24 +++++++++++++------
>  .../drm/i915/display/intel_display_types.h    |  9 ++++++-
>  drivers/gpu/drm/i915/display/intel_tv.c       |  4 ++--
>  drivers/gpu/drm/i915/display/vlv_dsi.c        |  6 ++---
>  drivers/gpu/drm/i915/i915_irq.c               |  4 ++--
>  7 files changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 99a25c0bb08f..4d6788ef2e5e 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1469,8 +1469,7 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
>  
>  	if (gen11_dsi_is_periodic_cmd_mode(intel_dsi))
> -		pipe_config->hw.adjusted_mode.private_flags |=
> -					I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
> +		pipe_config->mode_flags |= I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
>  }
>  
>  static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
> @@ -1555,10 +1554,6 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  
>  	pipe_config->port_clock = afe_clk(encoder, pipe_config) / 5;
>  
> -	/* We would not operate in periodic command mode */
> -	pipe_config->hw.adjusted_mode.private_flags &=
> -					~I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
> -

Since you delete this here, but not above (and then you could also detel
gen11_dsi_is_periodic_cmd_mode I think): It's dead code, maybe prep patch
to just garbage collect I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE?

I think the proper replacement is the mode flag stuff below, this is just
interim stuff that fell through the review.

With that prep patch to get rid of these 2 hunks above:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also surplus Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> on the patch
to delete I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE in case I miss the new
version.

>  	/*
>  	 * In case of TE GATE cmd mode, we
>  	 * receive TE from the slave if
> @@ -1566,14 +1561,14 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	 */
>  	if (is_cmd_mode(intel_dsi)) {
>  		if (intel_dsi->ports == (BIT(PORT_B) | BIT(PORT_A)))
> -			pipe_config->hw.adjusted_mode.private_flags |=
> +			pipe_config->mode_flags |=
>  						I915_MODE_FLAG_DSI_USE_TE1 |
>  						I915_MODE_FLAG_DSI_USE_TE0;
>  		else if (intel_dsi->ports == BIT(PORT_B))
> -			pipe_config->hw.adjusted_mode.private_flags |=
> +			pipe_config->mode_flags |=
>  						I915_MODE_FLAG_DSI_USE_TE1;
>  		else
> -			pipe_config->hw.adjusted_mode.private_flags |=
> +			pipe_config->mode_flags |=
>  						I915_MODE_FLAG_DSI_USE_TE0;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index d043057d2fa0..5863e339a426 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -252,6 +252,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  	crtc_state->wm.need_postvbl_update = false;
>  	crtc_state->fb_bits = 0;
>  	crtc_state->update_planes = 0;
> +	crtc_state->mode_flags &= ~I915_MODE_FLAG_INHERITED;
>  
>  	return &crtc_state->uapi;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index bcb5d754f20d..d88cade45c35 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6414,7 +6414,7 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
>  	 * forcibly enable IPS on the first fastset.
>  	 */
>  	if (new_crtc_state->update_pipe &&
> -	    old_crtc_state->hw.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED)
> +	    old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
>  		return true;
>  
>  	return !old_crtc_state->ips_enabled;
> @@ -13516,8 +13516,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	bool ret = true;
>  	u32 bp_gamma = 0;
>  	bool fixup_inherited = fastset &&
> -		(current_config->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> -		!(pipe_config->hw.mode.private_flags & I915_MODE_FLAG_INHERITED);
> +		(current_config->mode_flags & I915_MODE_FLAG_INHERITED) &&
> +		!(pipe_config->mode_flags & I915_MODE_FLAG_INHERITED);
>  
>  	if (fixup_inherited && !fastboot_enabled(dev_priv)) {
>  		drm_dbg_kms(&dev_priv->drm,
> @@ -14307,6 +14307,8 @@ intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state)
>  
>  	drm_calc_timestamping_constants(&crtc->base, adjusted_mode);
>  
> +	crtc->mode_flags = crtc_state->mode_flags;
> +
>  	/*
>  	 * The scanline counter increments at the leading edge of hsync.
>  	 *
> @@ -14668,8 +14670,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  	/* Catch I915_MODE_FLAG_INHERITED */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> -		if (new_crtc_state->uapi.mode.private_flags !=
> -		    old_crtc_state->uapi.mode.private_flags)
> +		if (new_crtc_state->mode_flags != old_crtc_state->mode_flags)
>  			new_crtc_state->uapi.mode_changed = true;
>  	}
>  
> @@ -15015,7 +15016,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>  	 * of enabling them on the CRTC's first fastset.
>  	 */
>  	if (new_crtc_state->update_pipe && !modeset &&
> -	    old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED)
> +	    old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
>  		intel_crtc_arm_fifo_underrun(crtc, new_crtc_state);
>  }
>  
> @@ -17486,6 +17487,15 @@ static int intel_initial_commit(struct drm_device *dev)
>  		}
>  
>  		if (crtc_state->hw.active) {
> +			/*
> +			 * We've not yet detected sink capabilities
> +			 * (audio,infoframes,etc.) and thus we don't want to
> +			 * force a full state recomputation yet. We want that to
> +			 * happen only for the first real commit from userspace.
> +			 * So preserve the inherited flag for the time being.
> +			 */
> +			crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
> +
>  			ret = drm_atomic_add_affected_planes(state, &crtc->base);
>  			if (ret)
>  				goto out;
> @@ -18256,7 +18266,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			 * set a flag to indicate that a full recalculation is
>  			 * needed on the next commit.
>  			 */
> -			mode->private_flags = I915_MODE_FLAG_INHERITED;
> +			crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
>  
>  			intel_crtc_compute_pixel_rate(crtc_state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bedd626c686..26df856f8b72 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -641,7 +641,7 @@ struct intel_crtc_scaler_state {
>  	int scaler_id;
>  };
>  
> -/* drm_mode->private_flags */
> +/* {crtc,crtc_state}->mode_flags */
>  #define I915_MODE_FLAG_INHERITED (1<<0)
>  /* Flag to get scanline using frame time stamps */
>  #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
> @@ -952,6 +952,9 @@ struct intel_crtc_state {
>  	/* Used by SDVO (and if we ever fix it, HDMI). */
>  	unsigned pixel_multiplier;
>  
> +	/* I915_MODE_FLAG_* */
> +	u8 mode_flags;
> +
>  	u8 lane_count;
>  
>  	/*
> @@ -1115,6 +1118,10 @@ struct intel_crtc {
>  	 */
>  	bool active;
>  	u8 plane_ids_mask;
> +
> +	/* I915_MODE_FLAG_* */
> +	u8 mode_flags;
> +
>  	unsigned long long enabled_power_domains;
>  	struct intel_overlay *overlay;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> index abc67207f2f3..777032d9697b 100644
> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> @@ -1158,7 +1158,7 @@ intel_tv_get_config(struct intel_encoder *encoder,
>  
>  	/* pixel counter doesn't work on i965gm TV output */
>  	if (IS_I965GM(dev_priv))
> -		adjusted_mode->private_flags |=
> +		pipe_config->mode_flags |=
>  			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>  }
>  
> @@ -1328,7 +1328,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>  
>  	/* pixel counter doesn't work on i965gm TV output */
>  	if (IS_I965GM(dev_priv))
> -		adjusted_mode->private_flags |=
> +		pipe_config->mode_flags |=
>  			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 4e18d4627065..d8b1c12cb21c 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -299,7 +299,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
>  
>  	if (IS_GEN9_LP(dev_priv)) {
>  		/* Enable Frame time stamp based scanline reporting */
> -		adjusted_mode->private_flags |=
> +		pipe_config->mode_flags |=
>  			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
>  
>  		/* Dual link goes to DSI transcoder A. */
> @@ -1098,8 +1098,8 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
>  	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
>  
>  	/* Enable Frame time stamo based scanline reporting */
> -	adjusted_mode->private_flags |=
> -			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
> +	pipe_config->mode_flags |=
> +		I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
>  
>  	/* In terms of pixels */
>  	adjusted_mode->crtc_hdisplay =
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1502ab44f1a5..55ed9516bfd3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -735,7 +735,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>  	vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)];
>  	mode = &vblank->hwmode;
>  
> -	if (mode->private_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
> +	if (crtc->mode_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
>  		return __intel_get_crtc_scanline_from_timestamp(crtc);
>  
>  	vtotal = mode->crtc_vtotal;
> @@ -794,7 +794,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
>  	unsigned long irqflags;
>  	bool use_scanline_counter = INTEL_GEN(dev_priv) >= 5 ||
>  		IS_G4X(dev_priv) || IS_GEN(dev_priv, 2) ||
> -		mode->private_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
> +		crtc->mode_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
>  
>  	if (drm_WARN_ON(&dev_priv->drm, !mode->crtc_clock)) {
>  		drm_dbg(&dev_priv->drm,
> -- 
> 2.24.1
>
Ville Syrjala April 7, 2020, 3:20 p.m. UTC | #2
On Tue, Apr 07, 2020 at 09:38:47AM +0200, Daniel Vetter wrote:
> On Fri, Apr 03, 2020 at 11:40:04PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Replace the use of mode->private_flags with a truly private bitmaks
> > in our own crtc state. We also need a copy in the crtc itself so the
> > vblank code can get at it. We already have scanline_offset in there
> > for a similar reason, as well as the vblank->hwmode which is assigned
> > via drm_calc_timestamping_constants(). Fortunately we now have a
> > nice place for doing the crtc_state->crtc copy in
> > intel_crtc_update_active_timings() which gets called both for
> > modesets and init/resume readout.
> > 
> > The one slightly iffy spot is the INHERITED flag which we want to
> > preserve until userspace/fb_helper does the first proper commit after
> > actually calling .detecti() on the connectors. Otherwise we don't have
> > the full sink capabilities (audio,infoframes,etc.) when .compute_config()
> > gets called and thus we will fail to enable those features when the
> > first userspace commit happens. The only internal commit we do prior to
> > that should be from intel_initial_commit() and there we can simply
> > preserve the INHERITED flag from the readout.
> > 
> > CC: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Emil Velikov <emil.l.velikov@gmail.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c        | 13 ++++------
> >  drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
> >  drivers/gpu/drm/i915/display/intel_display.c  | 24 +++++++++++++------
> >  .../drm/i915/display/intel_display_types.h    |  9 ++++++-
> >  drivers/gpu/drm/i915/display/intel_tv.c       |  4 ++--
> >  drivers/gpu/drm/i915/display/vlv_dsi.c        |  6 ++---
> >  drivers/gpu/drm/i915/i915_irq.c               |  4 ++--
> >  7 files changed, 37 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 99a25c0bb08f..4d6788ef2e5e 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1469,8 +1469,7 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
> >  	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
> >  
> >  	if (gen11_dsi_is_periodic_cmd_mode(intel_dsi))
> > -		pipe_config->hw.adjusted_mode.private_flags |=
> > -					I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
> > +		pipe_config->mode_flags |= I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
> >  }
> >  
> >  static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
> > @@ -1555,10 +1554,6 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
> >  
> >  	pipe_config->port_clock = afe_clk(encoder, pipe_config) / 5;
> >  
> > -	/* We would not operate in periodic command mode */
> > -	pipe_config->hw.adjusted_mode.private_flags &=
> > -					~I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
> > -
> 
> Since you delete this here, but not above (and then you could also detel
> gen11_dsi_is_periodic_cmd_mode I think): It's dead code, maybe prep patch
> to just garbage collect I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE?

I think this flag is still WIP. It was added very recently so I'm
assuming there is some plan for it (not that I like adding half
baked dead stuff like this). So we may want to wait a bit to see
where it's going. The reason I deleted this specific statement is
that we zero the crtc state before .compute_config() so this one
would remain dead code even if the flag starts to get used for
something.

> 
> I think the proper replacement is the mode flag stuff below, this is just
> interim stuff that fell through the review.
> 
> With that prep patch to get rid of these 2 hunks above:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Also surplus Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> on the patch
> to delete I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE in case I miss the new
> version.
>
Jani Nikula April 8, 2020, 9:34 a.m. UTC | #3
On Tue, 07 Apr 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Apr 07, 2020 at 09:38:47AM +0200, Daniel Vetter wrote:
>> On Fri, Apr 03, 2020 at 11:40:04PM +0300, Ville Syrjala wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > 
>> > Replace the use of mode->private_flags with a truly private bitmaks
>> > in our own crtc state. We also need a copy in the crtc itself so the
>> > vblank code can get at it. We already have scanline_offset in there
>> > for a similar reason, as well as the vblank->hwmode which is assigned
>> > via drm_calc_timestamping_constants(). Fortunately we now have a
>> > nice place for doing the crtc_state->crtc copy in
>> > intel_crtc_update_active_timings() which gets called both for
>> > modesets and init/resume readout.
>> > 
>> > The one slightly iffy spot is the INHERITED flag which we want to
>> > preserve until userspace/fb_helper does the first proper commit after
>> > actually calling .detecti() on the connectors. Otherwise we don't have
>> > the full sink capabilities (audio,infoframes,etc.) when .compute_config()
>> > gets called and thus we will fail to enable those features when the
>> > first userspace commit happens. The only internal commit we do prior to
>> > that should be from intel_initial_commit() and there we can simply
>> > preserve the INHERITED flag from the readout.
>> > 
>> > CC: Sam Ravnborg <sam@ravnborg.org>
>> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > Cc: Emil Velikov <emil.l.velikov@gmail.com>
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/icl_dsi.c        | 13 ++++------
>> >  drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
>> >  drivers/gpu/drm/i915/display/intel_display.c  | 24 +++++++++++++------
>> >  .../drm/i915/display/intel_display_types.h    |  9 ++++++-
>> >  drivers/gpu/drm/i915/display/intel_tv.c       |  4 ++--
>> >  drivers/gpu/drm/i915/display/vlv_dsi.c        |  6 ++---
>> >  drivers/gpu/drm/i915/i915_irq.c               |  4 ++--
>> >  7 files changed, 37 insertions(+), 24 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > index 99a25c0bb08f..4d6788ef2e5e 100644
>> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > @@ -1469,8 +1469,7 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>> >  	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
>> >  
>> >  	if (gen11_dsi_is_periodic_cmd_mode(intel_dsi))
>> > -		pipe_config->hw.adjusted_mode.private_flags |=
>> > -					I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
>> > +		pipe_config->mode_flags |= I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
>> >  }
>> >  
>> >  static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
>> > @@ -1555,10 +1554,6 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>> >  
>> >  	pipe_config->port_clock = afe_clk(encoder, pipe_config) / 5;
>> >  
>> > -	/* We would not operate in periodic command mode */
>> > -	pipe_config->hw.adjusted_mode.private_flags &=
>> > -					~I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
>> > -
>> 
>> Since you delete this here, but not above (and then you could also detel
>> gen11_dsi_is_periodic_cmd_mode I think): It's dead code, maybe prep patch
>> to just garbage collect I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE?
>
> I think this flag is still WIP. It was added very recently so I'm
> assuming there is some plan for it (not that I like adding half
> baked dead stuff like this). So we may want to wait a bit to see
> where it's going. The reason I deleted this specific statement is
> that we zero the crtc state before .compute_config() so this one
> would remain dead code even if the flag starts to get used for
> something.

It's part of a series that's halfway merged.

BR,
Jani.


>
>> 
>> I think the proper replacement is the mode flag stuff below, this is just
>> interim stuff that fell through the review.
>> 
>> With that prep patch to get rid of these 2 hunks above:
>> 
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> 
>> Also surplus Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> on the patch
>> to delete I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE in case I miss the new
>> version.
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 99a25c0bb08f..4d6788ef2e5e 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1469,8 +1469,7 @@  static void gen11_dsi_get_config(struct intel_encoder *encoder,
 	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
 
 	if (gen11_dsi_is_periodic_cmd_mode(intel_dsi))
-		pipe_config->hw.adjusted_mode.private_flags |=
-					I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
+		pipe_config->mode_flags |= I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
 }
 
 static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
@@ -1555,10 +1554,6 @@  static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->port_clock = afe_clk(encoder, pipe_config) / 5;
 
-	/* We would not operate in periodic command mode */
-	pipe_config->hw.adjusted_mode.private_flags &=
-					~I915_MODE_FLAG_DSI_PERIODIC_CMD_MODE;
-
 	/*
 	 * In case of TE GATE cmd mode, we
 	 * receive TE from the slave if
@@ -1566,14 +1561,14 @@  static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 	 */
 	if (is_cmd_mode(intel_dsi)) {
 		if (intel_dsi->ports == (BIT(PORT_B) | BIT(PORT_A)))
-			pipe_config->hw.adjusted_mode.private_flags |=
+			pipe_config->mode_flags |=
 						I915_MODE_FLAG_DSI_USE_TE1 |
 						I915_MODE_FLAG_DSI_USE_TE0;
 		else if (intel_dsi->ports == BIT(PORT_B))
-			pipe_config->hw.adjusted_mode.private_flags |=
+			pipe_config->mode_flags |=
 						I915_MODE_FLAG_DSI_USE_TE1;
 		else
-			pipe_config->hw.adjusted_mode.private_flags |=
+			pipe_config->mode_flags |=
 						I915_MODE_FLAG_DSI_USE_TE0;
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index d043057d2fa0..5863e339a426 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -252,6 +252,7 @@  intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->wm.need_postvbl_update = false;
 	crtc_state->fb_bits = 0;
 	crtc_state->update_planes = 0;
+	crtc_state->mode_flags &= ~I915_MODE_FLAG_INHERITED;
 
 	return &crtc_state->uapi;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index bcb5d754f20d..d88cade45c35 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6414,7 +6414,7 @@  static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
 	 * forcibly enable IPS on the first fastset.
 	 */
 	if (new_crtc_state->update_pipe &&
-	    old_crtc_state->hw.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED)
+	    old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
 		return true;
 
 	return !old_crtc_state->ips_enabled;
@@ -13516,8 +13516,8 @@  intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	bool ret = true;
 	u32 bp_gamma = 0;
 	bool fixup_inherited = fastset &&
-		(current_config->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
-		!(pipe_config->hw.mode.private_flags & I915_MODE_FLAG_INHERITED);
+		(current_config->mode_flags & I915_MODE_FLAG_INHERITED) &&
+		!(pipe_config->mode_flags & I915_MODE_FLAG_INHERITED);
 
 	if (fixup_inherited && !fastboot_enabled(dev_priv)) {
 		drm_dbg_kms(&dev_priv->drm,
@@ -14307,6 +14307,8 @@  intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state)
 
 	drm_calc_timestamping_constants(&crtc->base, adjusted_mode);
 
+	crtc->mode_flags = crtc_state->mode_flags;
+
 	/*
 	 * The scanline counter increments at the leading edge of hsync.
 	 *
@@ -14668,8 +14670,7 @@  static int intel_atomic_check(struct drm_device *dev,
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (new_crtc_state->uapi.mode.private_flags !=
-		    old_crtc_state->uapi.mode.private_flags)
+		if (new_crtc_state->mode_flags != old_crtc_state->mode_flags)
 			new_crtc_state->uapi.mode_changed = true;
 	}
 
@@ -15015,7 +15016,7 @@  static void intel_update_crtc(struct intel_atomic_state *state,
 	 * of enabling them on the CRTC's first fastset.
 	 */
 	if (new_crtc_state->update_pipe && !modeset &&
-	    old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED)
+	    old_crtc_state->mode_flags & I915_MODE_FLAG_INHERITED)
 		intel_crtc_arm_fifo_underrun(crtc, new_crtc_state);
 }
 
@@ -17486,6 +17487,15 @@  static int intel_initial_commit(struct drm_device *dev)
 		}
 
 		if (crtc_state->hw.active) {
+			/*
+			 * We've not yet detected sink capabilities
+			 * (audio,infoframes,etc.) and thus we don't want to
+			 * force a full state recomputation yet. We want that to
+			 * happen only for the first real commit from userspace.
+			 * So preserve the inherited flag for the time being.
+			 */
+			crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
+
 			ret = drm_atomic_add_affected_planes(state, &crtc->base);
 			if (ret)
 				goto out;
@@ -18256,7 +18266,7 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			 * set a flag to indicate that a full recalculation is
 			 * needed on the next commit.
 			 */
-			mode->private_flags = I915_MODE_FLAG_INHERITED;
+			crtc_state->mode_flags |= I915_MODE_FLAG_INHERITED;
 
 			intel_crtc_compute_pixel_rate(crtc_state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 2bedd626c686..26df856f8b72 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -641,7 +641,7 @@  struct intel_crtc_scaler_state {
 	int scaler_id;
 };
 
-/* drm_mode->private_flags */
+/* {crtc,crtc_state}->mode_flags */
 #define I915_MODE_FLAG_INHERITED (1<<0)
 /* Flag to get scanline using frame time stamps */
 #define I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP (1<<1)
@@ -952,6 +952,9 @@  struct intel_crtc_state {
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
 
+	/* I915_MODE_FLAG_* */
+	u8 mode_flags;
+
 	u8 lane_count;
 
 	/*
@@ -1115,6 +1118,10 @@  struct intel_crtc {
 	 */
 	bool active;
 	u8 plane_ids_mask;
+
+	/* I915_MODE_FLAG_* */
+	u8 mode_flags;
+
 	unsigned long long enabled_power_domains;
 	struct intel_overlay *overlay;
 
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
index abc67207f2f3..777032d9697b 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -1158,7 +1158,7 @@  intel_tv_get_config(struct intel_encoder *encoder,
 
 	/* pixel counter doesn't work on i965gm TV output */
 	if (IS_I965GM(dev_priv))
-		adjusted_mode->private_flags |=
+		pipe_config->mode_flags |=
 			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
 }
 
@@ -1328,7 +1328,7 @@  intel_tv_compute_config(struct intel_encoder *encoder,
 
 	/* pixel counter doesn't work on i965gm TV output */
 	if (IS_I965GM(dev_priv))
-		adjusted_mode->private_flags |=
+		pipe_config->mode_flags |=
 			I915_MODE_FLAG_USE_SCANLINE_COUNTER;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 4e18d4627065..d8b1c12cb21c 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -299,7 +299,7 @@  static int intel_dsi_compute_config(struct intel_encoder *encoder,
 
 	if (IS_GEN9_LP(dev_priv)) {
 		/* Enable Frame time stamp based scanline reporting */
-		adjusted_mode->private_flags |=
+		pipe_config->mode_flags |=
 			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
 
 		/* Dual link goes to DSI transcoder A. */
@@ -1098,8 +1098,8 @@  static void bxt_dsi_get_pipe_config(struct intel_encoder *encoder,
 	pipe_config->pipe_bpp = bdw_get_pipemisc_bpp(crtc);
 
 	/* Enable Frame time stamo based scanline reporting */
-	adjusted_mode->private_flags |=
-			I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
+	pipe_config->mode_flags |=
+		I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP;
 
 	/* In terms of pixels */
 	adjusted_mode->crtc_hdisplay =
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1502ab44f1a5..55ed9516bfd3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -735,7 +735,7 @@  static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 	vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)];
 	mode = &vblank->hwmode;
 
-	if (mode->private_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
+	if (crtc->mode_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
 		return __intel_get_crtc_scanline_from_timestamp(crtc);
 
 	vtotal = mode->crtc_vtotal;
@@ -794,7 +794,7 @@  static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 	unsigned long irqflags;
 	bool use_scanline_counter = INTEL_GEN(dev_priv) >= 5 ||
 		IS_G4X(dev_priv) || IS_GEN(dev_priv, 2) ||
-		mode->private_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
+		crtc->mode_flags & I915_MODE_FLAG_USE_SCANLINE_COUNTER;
 
 	if (drm_WARN_ON(&dev_priv->drm, !mode->crtc_clock)) {
 		drm_dbg(&dev_priv->drm,