[Intel-gfx] [PATCH 00/12] drm/i915: Move bigjoiner refactoring

[Intel-gfx] [PATCH 00/12] drm/i915: Move bigjoiner refactoring

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This is an attempt at more or less finish the bigjoiner
state computation/readout refactoring.

Stuff that should now be in decent shape:
- cursor should appear in the right spot on all pipes
- plane clipping/etc. independent of number of joined pipes
  thanks to the PIPESRC drm_rect
- the PIPESRC drm_rect should prove helpful for the seam
  elimination stuff too in the future, as well as for some
  other planned scaler fixes/cleanups
- bigjoiner vs. MSO timings should be properly handled now

What is likely still busted:
- panel fitter. The state computation needs to be redesigned fully
  for bigjoiner. Semi-related to the aforementioned scaler work.
- the modeset sequence is still a huge mess. That will have
  to be the next major refactoring target I think.

Pushed the lot here:
git://github.com/vsyrjala/linux.git pipesrc_rect_3

Ville Syrjälä (12):
  drm/i915: Fix cursor coordinates on bigjoiner slave
  drm/i915: Remove nop bigjoiner state copy
  drm/i915: Rename variables in intel_crtc_compute_config()
  drm/i915: Extract intel_splitter_adjust_timings()
  drm/i915: Extract intel_bigjoiner_adjust_timings()
  drm/i915: Extract intel_crtc_compute_pipe_src()
  drm/i915: Extract intel_crtc_compute_pipe_mode()
  drm/i915: Fix MSO vs. bigjoiner timings confusion
  drm/i915: Start tracking PIPESRC as a drm_rect
  drm/i915: Eliminate bigjoiner boolean
  drm/i915: Use bigjoiner_pipes more
  drm/i915: Make the PIPESC rect relative to the entire bigjoiner area

 .../gpu/drm/i915/display/intel_atomic_plane.c |  20 +-
 drivers/gpu/drm/i915/display/intel_cursor.c   |   7 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 350 +++++++++++-------
 .../drm/i915/display/intel_display_debugfs.c  |   6 +-
 .../drm/i915/display/intel_display_types.h    |   5 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  13 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  |  22 +-
 drivers/gpu/drm/i915/display/intel_panel.c    |  70 ++--
 drivers/gpu/drm/i915/display/intel_vdsc.c     |   8 +-
 drivers/gpu/drm/i915/display/skl_scaler.c     |  12 +-
 .../drm/i915/display/skl_universal_plane.c    |   4 +-
 11 files changed, 294 insertions(+), 223 deletions(-)

-- 
2.34.1

[Intel-gfx] [PATCH 01/12] drm/i915: Fix cursor coordinates on bigjoiner slave

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Adjust the cursor dst coordinates appripriately when it's on
the bigjoiner slave pipe. intel_atomic_plane_check_clipping()
already did this but with the cursor we discard those results
(apart from uapi.visible and error checks) since the hardware
will be doing the clipping for us.

v2: Rebase due to bigjoiner bitmask usage

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 2ade8fdd9bdd..3e80763aa828 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -152,6 +152,9 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	/* Use the unclipped src/dst rectangles, which we program to hw */
 	plane_state->uapi.src = src;
 	plane_state->uapi.dst = dst;
+	if (intel_crtc_is_bigjoiner_slave(crtc_state))
+		drm_rect_translate(&plane_state->uapi.dst,
+				   -crtc_state->pipe_src_w, 0);
 
 	ret = intel_cursor_check_surface(plane_state);
 	if (ret)
-- 
2.34.1

Re: [Intel-gfx] [PATCH 01/12] drm/i915: Fix cursor coordinates on bigjoiner slave

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:31:57PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Adjust the cursor dst coordinates appripriately when it's on
> the bigjoiner slave pipe. intel_atomic_plane_check_clipping()
> already did this but with the cursor we discard those results
> (apart from uapi.visible and error checks) since the hardware
> will be doing the clipping for us.
> 
> v2: Rebase due to bigjoiner bitmask usage
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 2ade8fdd9bdd..3e80763aa828 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -152,6 +152,9 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  	/* Use the unclipped src/dst rectangles, which we program to hw */
>  	plane_state->uapi.src = src;
>  	plane_state->uapi.dst = dst;
> +	if (intel_crtc_is_bigjoiner_slave(crtc_state))
> +		drm_rect_translate(&plane_state->uapi.dst,
> +				   -crtc_state->pipe_src_w, 0);

So this is basically to offset the cursor position from say 3860 to 0, 3861 to 1 ....7680 to 3860 for the right half
of the screen right?
And without this, it will just keep it at first rectangle?

Manasi

>  
>  	ret = intel_cursor_check_surface(plane_state);
>  	if (ret)
> -- 
> 2.34.1
> 

Re: [Intel-gfx] [PATCH 01/12] drm/i915: Fix cursor coordinates on bigjoiner slave

[Ville Syrjälä]

On Tue, Feb 15, 2022 at 07:25:36PM -0800, Navare, Manasi wrote:
> On Tue, Feb 15, 2022 at 08:31:57PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Adjust the cursor dst coordinates appripriately when it's on
> > the bigjoiner slave pipe. intel_atomic_plane_check_clipping()
> > already did this but with the cursor we discard those results
> > (apart from uapi.visible and error checks) since the hardware
> > will be doing the clipping for us.
> > 
> > v2: Rebase due to bigjoiner bitmask usage
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cursor.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> > index 2ade8fdd9bdd..3e80763aa828 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> > @@ -152,6 +152,9 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> >  	/* Use the unclipped src/dst rectangles, which we program to hw */
> >  	plane_state->uapi.src = src;
> >  	plane_state->uapi.dst = dst;
> > +	if (intel_crtc_is_bigjoiner_slave(crtc_state))
> > +		drm_rect_translate(&plane_state->uapi.dst,
> > +				   -crtc_state->pipe_src_w, 0);
> 
> So this is basically to offset the cursor position from say 3860 to 0, 3861 to 1 ....7680 to 3860 for the right half
> of the screen right?
> And without this, it will just keep it at first rectangle?

Yes. The original rectangle came from userspace and is thus
relative to the whole area covered by all the joined pipes.
But the plane coordinates we hand to the hardware must be
relative to their respective pipe.

It's a bit ugly as is but works for now. The last patch make
this much nicer by removing all the assumptions here about
the relative positions of the pipes.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 01/12] drm/i915: Fix cursor coordinates on bigjoiner slave

[Navare, Manasi]

On Wed, Feb 16, 2022 at 10:39:56AM +0200, Ville Syrjälä wrote:
> On Tue, Feb 15, 2022 at 07:25:36PM -0800, Navare, Manasi wrote:
> > On Tue, Feb 15, 2022 at 08:31:57PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Adjust the cursor dst coordinates appripriately when it's on
> > > the bigjoiner slave pipe. intel_atomic_plane_check_clipping()
> > > already did this but with the cursor we discard those results
> > > (apart from uapi.visible and error checks) since the hardware
> > > will be doing the clipping for us.
> > > 
> > > v2: Rebase due to bigjoiner bitmask usage
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cursor.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> > > index 2ade8fdd9bdd..3e80763aa828 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> > > @@ -152,6 +152,9 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> > >  	/* Use the unclipped src/dst rectangles, which we program to hw */
> > >  	plane_state->uapi.src = src;
> > >  	plane_state->uapi.dst = dst;
> > > +	if (intel_crtc_is_bigjoiner_slave(crtc_state))
> > > +		drm_rect_translate(&plane_state->uapi.dst,
> > > +				   -crtc_state->pipe_src_w, 0);
> > 
> > So this is basically to offset the cursor position from say 3860 to 0, 3861 to 1 ....7680 to 3860 for the right half
> > of the screen right?
> > And without this, it will just keep it at first rectangle?
> 
> Yes. The original rectangle came from userspace and is thus
> relative to the whole area covered by all the joined pipes.
> But the plane coordinates we hand to the hardware must be
> relative to their respective pipe.
> 
> It's a bit ugly as is but works for now. The last patch make
> this much nicer by removing all the assumptions here about
> the relative positions of the pipes.

Okay yes makes sens,

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> 
> -- 
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH 02/12] drm/i915: Remove nop bigjoiner state copy

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We just copied over the whole master crtc state, including
cpu_transcoder+has_audio. No need to copy those again.

Also get rid of the unhelpful comment.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 622903847551..49ca13abd108 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5779,12 +5779,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
 
 	copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc);
 
-	/* Some fixups */
 	slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed;
 	slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
 	slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
-	slave_crtc_state->cpu_transcoder = master_crtc_state->cpu_transcoder;
-	slave_crtc_state->has_audio = master_crtc_state->has_audio;
 
 	return 0;
 }
-- 
2.34.1

Re: [Intel-gfx] [PATCH 02/12] drm/i915: Remove nop bigjoiner state copy

[Nautiyal, Ankit K]

On 2/16/2022 12:01 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We just copied over the whole master crtc state, including
> cpu_transcoder+has_audio. No need to copy those again.
>
> Also get rid of the unhelpful comment.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display.c | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 622903847551..49ca13abd108 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5779,12 +5779,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
>   
>   	copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc);
>   
> -	/* Some fixups */
>   	slave_crtc_state->uapi.mode_changed = master_crtc_state->uapi.mode_changed;
>   	slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
>   	slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
> -	slave_crtc_state->cpu_transcoder = master_crtc_state->cpu_transcoder;
> -	slave_crtc_state->has_audio = master_crtc_state->has_audio;

Makes sense, these two are only set, uapi was preserved from older slave 
state, so only the relevant bits need to be copied from master here.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>




>   
>   	return 0;
>   }

[Intel-gfx] [PATCH 03/12] drm/i915: Rename variables in intel_crtc_compute_config()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do the s/dev_priv/i915/ and s/pipe_config/crtc_state/ renames
to intel_crtc_compute_config(). I want to start splitting this
up a bit and doing the renames now avoids spreading these old
nameing conventions elsewhere.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 50 ++++++++++----------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 49ca13abd108..5da8db3dda8f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2787,16 +2787,16 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
 }
 
 static int intel_crtc_compute_config(struct intel_crtc *crtc,
-				     struct intel_crtc_state *pipe_config)
+				     struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct drm_display_mode *pipe_mode = &pipe_config->hw.pipe_mode;
-	int clock_limit = dev_priv->max_dotclk_freq;
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
+	int clock_limit = i915->max_dotclk_freq;
 
-	drm_mode_copy(pipe_mode, &pipe_config->hw.adjusted_mode);
+	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
 
 	/* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
-	if (pipe_config->bigjoiner) {
+	if (crtc_state->bigjoiner) {
 		pipe_mode->crtc_clock /= 2;
 		pipe_mode->crtc_hdisplay /= 2;
 		pipe_mode->crtc_hblank_start /= 2;
@@ -2804,12 +2804,12 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		pipe_mode->crtc_hsync_start /= 2;
 		pipe_mode->crtc_hsync_end /= 2;
 		pipe_mode->crtc_htotal /= 2;
-		pipe_config->pipe_src_w /= 2;
+		crtc_state->pipe_src_w /= 2;
 	}
 
-	if (pipe_config->splitter.enable) {
-		int n = pipe_config->splitter.link_count;
-		int overlap = pipe_config->splitter.pixel_overlap;
+	if (crtc_state->splitter.enable) {
+		int n = crtc_state->splitter.link_count;
+		int overlap = crtc_state->splitter.pixel_overlap;
 
 		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
 		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
@@ -2822,8 +2822,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
 
-	if (DISPLAY_VER(dev_priv) < 4) {
-		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
+	if (DISPLAY_VER(i915) < 4) {
+		clock_limit = i915->max_cdclk_freq * 9 / 10;
 
 		/*
 		 * Enable double wide mode when the dot clock
@@ -2831,16 +2831,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		 */
 		if (intel_crtc_supports_double_wide(crtc) &&
 		    pipe_mode->crtc_clock > clock_limit) {
-			clock_limit = dev_priv->max_dotclk_freq;
-			pipe_config->double_wide = true;
+			clock_limit = i915->max_dotclk_freq;
+			crtc_state->double_wide = true;
 		}
 	}
 
 	if (pipe_mode->crtc_clock > clock_limit) {
-		drm_dbg_kms(&dev_priv->drm,
+		drm_dbg_kms(&i915->drm,
 			    "requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
 			    pipe_mode->crtc_clock, clock_limit,
-			    yesno(pipe_config->double_wide));
+			    yesno(crtc_state->double_wide));
 		return -EINVAL;
 	}
 
@@ -2850,25 +2850,25 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	 * - LVDS dual channel mode
 	 * - Double wide pipe
 	 */
-	if (pipe_config->pipe_src_w & 1) {
-		if (pipe_config->double_wide) {
-			drm_dbg_kms(&dev_priv->drm,
+	if (crtc_state->pipe_src_w & 1) {
+		if (crtc_state->double_wide) {
+			drm_dbg_kms(&i915->drm,
 				    "Odd pipe source width not supported with double wide pipe\n");
 			return -EINVAL;
 		}
 
-		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_LVDS) &&
-		    intel_is_dual_link_lvds(dev_priv)) {
-			drm_dbg_kms(&dev_priv->drm,
+		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+		    intel_is_dual_link_lvds(i915)) {
+			drm_dbg_kms(&i915->drm,
 				    "Odd pipe source width not supported with dual link LVDS\n");
 			return -EINVAL;
 		}
 	}
 
-	intel_crtc_compute_pixel_rate(pipe_config);
+	intel_crtc_compute_pixel_rate(crtc_state);
 
-	if (pipe_config->has_pch_encoder)
-		return ilk_fdi_compute_config(crtc, pipe_config);
+	if (crtc_state->has_pch_encoder)
+		return ilk_fdi_compute_config(crtc, crtc_state);
 
 	return 0;
 }
-- 
2.34.1

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Rename variables in intel_crtc_compute_config()

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:31:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do the s/dev_priv/i915/ and s/pipe_config/crtc_state/ renames
> to intel_crtc_compute_config(). I want to start splitting this
> up a bit and doing the renames now avoids spreading these old
> nameing conventions elsewhere.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yes the name cleanup looks good, would be good to call out "No functional changes" in the
commit message.

With that

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 50 ++++++++++----------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 49ca13abd108..5da8db3dda8f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2787,16 +2787,16 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
>  }
>  
>  static int intel_crtc_compute_config(struct intel_crtc *crtc,
> -				     struct intel_crtc_state *pipe_config)
> +				     struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	struct drm_display_mode *pipe_mode = &pipe_config->hw.pipe_mode;
> -	int clock_limit = dev_priv->max_dotclk_freq;
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
> +	int clock_limit = i915->max_dotclk_freq;
>  
> -	drm_mode_copy(pipe_mode, &pipe_config->hw.adjusted_mode);
> +	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
>  
>  	/* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
> -	if (pipe_config->bigjoiner) {
> +	if (crtc_state->bigjoiner) {
>  		pipe_mode->crtc_clock /= 2;
>  		pipe_mode->crtc_hdisplay /= 2;
>  		pipe_mode->crtc_hblank_start /= 2;
> @@ -2804,12 +2804,12 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		pipe_mode->crtc_hsync_start /= 2;
>  		pipe_mode->crtc_hsync_end /= 2;
>  		pipe_mode->crtc_htotal /= 2;
> -		pipe_config->pipe_src_w /= 2;
> +		crtc_state->pipe_src_w /= 2;
>  	}
>  
> -	if (pipe_config->splitter.enable) {
> -		int n = pipe_config->splitter.link_count;
> -		int overlap = pipe_config->splitter.pixel_overlap;
> +	if (crtc_state->splitter.enable) {
> +		int n = crtc_state->splitter.link_count;
> +		int overlap = crtc_state->splitter.pixel_overlap;
>  
>  		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
>  		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
> @@ -2822,8 +2822,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
>  
> -	if (DISPLAY_VER(dev_priv) < 4) {
> -		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> +	if (DISPLAY_VER(i915) < 4) {
> +		clock_limit = i915->max_cdclk_freq * 9 / 10;
>  
>  		/*
>  		 * Enable double wide mode when the dot clock
> @@ -2831,16 +2831,16 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		 */
>  		if (intel_crtc_supports_double_wide(crtc) &&
>  		    pipe_mode->crtc_clock > clock_limit) {
> -			clock_limit = dev_priv->max_dotclk_freq;
> -			pipe_config->double_wide = true;
> +			clock_limit = i915->max_dotclk_freq;
> +			crtc_state->double_wide = true;
>  		}
>  	}
>  
>  	if (pipe_mode->crtc_clock > clock_limit) {
> -		drm_dbg_kms(&dev_priv->drm,
> +		drm_dbg_kms(&i915->drm,
>  			    "requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
>  			    pipe_mode->crtc_clock, clock_limit,
> -			    yesno(pipe_config->double_wide));
> +			    yesno(crtc_state->double_wide));
>  		return -EINVAL;
>  	}
>  
> @@ -2850,25 +2850,25 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  	 * - LVDS dual channel mode
>  	 * - Double wide pipe
>  	 */
> -	if (pipe_config->pipe_src_w & 1) {
> -		if (pipe_config->double_wide) {
> -			drm_dbg_kms(&dev_priv->drm,
> +	if (crtc_state->pipe_src_w & 1) {
> +		if (crtc_state->double_wide) {
> +			drm_dbg_kms(&i915->drm,
>  				    "Odd pipe source width not supported with double wide pipe\n");
>  			return -EINVAL;
>  		}
>  
> -		if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_LVDS) &&
> -		    intel_is_dual_link_lvds(dev_priv)) {
> -			drm_dbg_kms(&dev_priv->drm,
> +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> +		    intel_is_dual_link_lvds(i915)) {
> +			drm_dbg_kms(&i915->drm,
>  				    "Odd pipe source width not supported with dual link LVDS\n");
>  			return -EINVAL;
>  		}
>  	}
>  
> -	intel_crtc_compute_pixel_rate(pipe_config);
> +	intel_crtc_compute_pixel_rate(crtc_state);
>  
> -	if (pipe_config->has_pch_encoder)
> -		return ilk_fdi_compute_config(crtc, pipe_config);
> +	if (crtc_state->has_pch_encoder)
> +		return ilk_fdi_compute_config(crtc, crtc_state);
>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 04/12] drm/i915: Extract intel_splitter_adjust_timings()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's not replicate the same piece of code to expand
the MSO segment timings to full width in many places.
Pull it into a helper

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 54 ++++++++++----------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5da8db3dda8f..70017526fa81 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2724,6 +2724,30 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
 			ilk_pipe_pixel_rate(crtc_state);
 }
 
+static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
+					  struct drm_display_mode *mode)
+{
+	int overlap = crtc_state->splitter.pixel_overlap;
+	int n = crtc_state->splitter.link_count;
+
+	if (!crtc_state->splitter.enable)
+		return;
+
+	/*
+	 * eDP MSO uses segment timings from EDID for transcoder
+	 * timings, but full mode for everything else.
+	 *
+	 * h_full = (h_segment - pixel_overlap) * link_count
+	 */
+	mode->crtc_hdisplay = (mode->crtc_hdisplay - overlap) * n;
+	mode->crtc_hblank_start = (mode->crtc_hblank_start - overlap) * n;
+	mode->crtc_hblank_end = (mode->crtc_hblank_end - overlap) * n;
+	mode->crtc_hsync_start = (mode->crtc_hsync_start - overlap) * n;
+	mode->crtc_hsync_end = (mode->crtc_hsync_end - overlap) * n;
+	mode->crtc_htotal = (mode->crtc_htotal - overlap) * n;
+	mode->crtc_clock *= n;
+}
+
 static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state)
 {
 	struct drm_display_mode *mode = &crtc_state->hw.mode;
@@ -2747,22 +2771,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
 	}
 
 	if (crtc_state->splitter.enable) {
-		int n = crtc_state->splitter.link_count;
-		int overlap = crtc_state->splitter.pixel_overlap;
-
-		/*
-		 * eDP MSO uses segment timings from EDID for transcoder
-		 * timings, but full mode for everything else.
-		 *
-		 * h_full = (h_segment - pixel_overlap) * link_count
-		 */
-		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
-		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
-		pipe_mode->crtc_hblank_end = (pipe_mode->crtc_hblank_end - overlap) * n;
-		pipe_mode->crtc_hsync_start = (pipe_mode->crtc_hsync_start - overlap) * n;
-		pipe_mode->crtc_hsync_end = (pipe_mode->crtc_hsync_end - overlap) * n;
-		pipe_mode->crtc_htotal = (pipe_mode->crtc_htotal - overlap) * n;
-		pipe_mode->crtc_clock *= n;
+		intel_splitter_adjust_timings(crtc_state, pipe_mode);
 
 		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
 		intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
@@ -2807,18 +2816,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		crtc_state->pipe_src_w /= 2;
 	}
 
-	if (crtc_state->splitter.enable) {
-		int n = crtc_state->splitter.link_count;
-		int overlap = crtc_state->splitter.pixel_overlap;
-
-		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
-		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
-		pipe_mode->crtc_hblank_end = (pipe_mode->crtc_hblank_end - overlap) * n;
-		pipe_mode->crtc_hsync_start = (pipe_mode->crtc_hsync_start - overlap) * n;
-		pipe_mode->crtc_hsync_end = (pipe_mode->crtc_hsync_end - overlap) * n;
-		pipe_mode->crtc_htotal = (pipe_mode->crtc_htotal - overlap) * n;
-		pipe_mode->crtc_clock *= n;
-	}
+	intel_splitter_adjust_timings(crtc_state, pipe_mode);
 
 	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
 
-- 
2.34.1

Re: [Intel-gfx] [PATCH 04/12] drm/i915: Extract intel_splitter_adjust_timings()

[Jani Nikula]

On Tue, 15 Feb 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's not replicate the same piece of code to expand
> the MSO segment timings to full width in many places.
> Pull it into a helper

Did I duplicate that? Yuck.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 54 ++++++++++----------
>  1 file changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5da8db3dda8f..70017526fa81 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2724,6 +2724,30 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  			ilk_pipe_pixel_rate(crtc_state);
>  }
>  
> +static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
> +					  struct drm_display_mode *mode)
> +{
> +	int overlap = crtc_state->splitter.pixel_overlap;
> +	int n = crtc_state->splitter.link_count;
> +
> +	if (!crtc_state->splitter.enable)
> +		return;
> +
> +	/*
> +	 * eDP MSO uses segment timings from EDID for transcoder
> +	 * timings, but full mode for everything else.
> +	 *
> +	 * h_full = (h_segment - pixel_overlap) * link_count
> +	 */
> +	mode->crtc_hdisplay = (mode->crtc_hdisplay - overlap) * n;
> +	mode->crtc_hblank_start = (mode->crtc_hblank_start - overlap) * n;
> +	mode->crtc_hblank_end = (mode->crtc_hblank_end - overlap) * n;
> +	mode->crtc_hsync_start = (mode->crtc_hsync_start - overlap) * n;
> +	mode->crtc_hsync_end = (mode->crtc_hsync_end - overlap) * n;
> +	mode->crtc_htotal = (mode->crtc_htotal - overlap) * n;
> +	mode->crtc_clock *= n;
> +}
> +
>  static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_display_mode *mode = &crtc_state->hw.mode;
> @@ -2747,22 +2771,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  	}
>  
>  	if (crtc_state->splitter.enable) {
> -		int n = crtc_state->splitter.link_count;
> -		int overlap = crtc_state->splitter.pixel_overlap;
> -
> -		/*
> -		 * eDP MSO uses segment timings from EDID for transcoder
> -		 * timings, but full mode for everything else.
> -		 *
> -		 * h_full = (h_segment - pixel_overlap) * link_count
> -		 */
> -		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
> -		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
> -		pipe_mode->crtc_hblank_end = (pipe_mode->crtc_hblank_end - overlap) * n;
> -		pipe_mode->crtc_hsync_start = (pipe_mode->crtc_hsync_start - overlap) * n;
> -		pipe_mode->crtc_hsync_end = (pipe_mode->crtc_hsync_end - overlap) * n;
> -		pipe_mode->crtc_htotal = (pipe_mode->crtc_htotal - overlap) * n;
> -		pipe_mode->crtc_clock *= n;
> +		intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
>  		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
>  		intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
> @@ -2807,18 +2816,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		crtc_state->pipe_src_w /= 2;
>  	}
>  
> -	if (crtc_state->splitter.enable) {
> -		int n = crtc_state->splitter.link_count;
> -		int overlap = crtc_state->splitter.pixel_overlap;
> -
> -		pipe_mode->crtc_hdisplay = (pipe_mode->crtc_hdisplay - overlap) * n;
> -		pipe_mode->crtc_hblank_start = (pipe_mode->crtc_hblank_start - overlap) * n;
> -		pipe_mode->crtc_hblank_end = (pipe_mode->crtc_hblank_end - overlap) * n;
> -		pipe_mode->crtc_hsync_start = (pipe_mode->crtc_hsync_start - overlap) * n;
> -		pipe_mode->crtc_hsync_end = (pipe_mode->crtc_hsync_end - overlap) * n;
> -		pipe_mode->crtc_htotal = (pipe_mode->crtc_htotal - overlap) * n;
> -		pipe_mode->crtc_clock *= n;
> -	}
> +	intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
>  	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH 05/12] drm/i915: Extract intel_bigjoiner_adjust_timings()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Deduplicate the code to convert the full timings to
per-pipe timings for bigjoiner usage.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 41 +++++++++-----------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 70017526fa81..19417ff975c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2724,6 +2724,21 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
 			ilk_pipe_pixel_rate(crtc_state);
 }
 
+static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
+					   struct drm_display_mode *mode)
+{
+	if (!crtc_state->bigjoiner)
+		return;
+
+	mode->crtc_clock /= 2;
+	mode->crtc_hdisplay /= 2;
+	mode->crtc_hblank_start /= 2;
+	mode->crtc_hblank_end /= 2;
+	mode->crtc_hsync_start /= 2;
+	mode->crtc_hsync_end /= 2;
+	mode->crtc_htotal /= 2;
+}
+
 static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
 					  struct drm_display_mode *mode)
 {
@@ -2756,19 +2771,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
 
 	drm_mode_copy(pipe_mode, adjusted_mode);
 
-	if (crtc_state->bigjoiner) {
-		/*
-		 * transcoder is programmed to the full mode,
-		 * but pipe timings are half of the transcoder mode
-		 */
-		pipe_mode->crtc_hdisplay /= 2;
-		pipe_mode->crtc_hblank_start /= 2;
-		pipe_mode->crtc_hblank_end /= 2;
-		pipe_mode->crtc_hsync_start /= 2;
-		pipe_mode->crtc_hsync_end /= 2;
-		pipe_mode->crtc_htotal /= 2;
-		pipe_mode->crtc_clock /= 2;
-	}
+	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
 
 	if (crtc_state->splitter.enable) {
 		intel_splitter_adjust_timings(crtc_state, pipe_mode);
@@ -2804,17 +2807,9 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
 
-	/* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
-	if (crtc_state->bigjoiner) {
-		pipe_mode->crtc_clock /= 2;
-		pipe_mode->crtc_hdisplay /= 2;
-		pipe_mode->crtc_hblank_start /= 2;
-		pipe_mode->crtc_hblank_end /= 2;
-		pipe_mode->crtc_hsync_start /= 2;
-		pipe_mode->crtc_hsync_end /= 2;
-		pipe_mode->crtc_htotal /= 2;
+	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
+	if (crtc_state->bigjoiner)
 		crtc_state->pipe_src_w /= 2;
-	}
 
 	intel_splitter_adjust_timings(crtc_state, pipe_mode);
 
-- 
2.34.1

Re: [Intel-gfx] [PATCH 05/12] drm/i915: Extract intel_bigjoiner_adjust_timings()

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:01PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Deduplicate the code to convert the full timings to
> per-pipe timings for bigjoiner usage.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes sense to have a helper to do this:

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 41 +++++++++-----------
>  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 70017526fa81..19417ff975c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2724,6 +2724,21 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  			ilk_pipe_pixel_rate(crtc_state);
>  }
>  
> +static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
> +					   struct drm_display_mode *mode)
> +{
> +	if (!crtc_state->bigjoiner)
> +		return;
> +
> +	mode->crtc_clock /= 2;
> +	mode->crtc_hdisplay /= 2;
> +	mode->crtc_hblank_start /= 2;
> +	mode->crtc_hblank_end /= 2;
> +	mode->crtc_hsync_start /= 2;
> +	mode->crtc_hsync_end /= 2;
> +	mode->crtc_htotal /= 2;
> +}
> +
>  static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
>  					  struct drm_display_mode *mode)
>  {
> @@ -2756,19 +2771,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  
>  	drm_mode_copy(pipe_mode, adjusted_mode);
>  
> -	if (crtc_state->bigjoiner) {
> -		/*
> -		 * transcoder is programmed to the full mode,
> -		 * but pipe timings are half of the transcoder mode
> -		 */
> -		pipe_mode->crtc_hdisplay /= 2;
> -		pipe_mode->crtc_hblank_start /= 2;
> -		pipe_mode->crtc_hblank_end /= 2;
> -		pipe_mode->crtc_hsync_start /= 2;
> -		pipe_mode->crtc_hsync_end /= 2;
> -		pipe_mode->crtc_htotal /= 2;
> -		pipe_mode->crtc_clock /= 2;
> -	}
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
>  
>  	if (crtc_state->splitter.enable) {
>  		intel_splitter_adjust_timings(crtc_state, pipe_mode);
> @@ -2804,17 +2807,9 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
>  
> -	/* Adjust pipe_mode for bigjoiner, with half the horizontal mode */
> -	if (crtc_state->bigjoiner) {
> -		pipe_mode->crtc_clock /= 2;
> -		pipe_mode->crtc_hdisplay /= 2;
> -		pipe_mode->crtc_hblank_start /= 2;
> -		pipe_mode->crtc_hblank_end /= 2;
> -		pipe_mode->crtc_hsync_start /= 2;
> -		pipe_mode->crtc_hsync_end /= 2;
> -		pipe_mode->crtc_htotal /= 2;
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> +	if (crtc_state->bigjoiner)
>  		crtc_state->pipe_src_w /= 2;
> -	}
>  
>  	intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 06/12] drm/i915: Extract intel_crtc_compute_pipe_src()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_crtc_compute_config() doesn't really tell a unified story.
Let's chunk it up into pieces. We'll start with
intel_crtc_compute_pipe_src().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 62 ++++++++++++--------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 19417ff975c6..3d3fddd3f452 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2798,18 +2798,55 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
 	intel_crtc_readout_derived_state(crtc_state);
 }
 
+static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+	if (crtc_state->bigjoiner)
+		crtc_state->pipe_src_w /= 2;
+
+	/*
+	 * Pipe horizontal size must be even in:
+	 * - DVO ganged mode
+	 * - LVDS dual channel mode
+	 * - Double wide pipe
+	 */
+	if (crtc_state->pipe_src_w & 1) {
+		if (crtc_state->double_wide) {
+			drm_dbg_kms(&i915->drm,
+				    "[CRTC:%d:%s] Odd pipe source width not supported with double wide pipe\n",
+				    crtc->base.base.id, crtc->base.name);
+			return -EINVAL;
+		}
+
+		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+		    intel_is_dual_link_lvds(i915)) {
+			drm_dbg_kms(&i915->drm,
+				    "[CRTC:%d:%s] Odd pipe source width not supported with dual link LVDS\n",
+				    crtc->base.base.id, crtc->base.name);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int intel_crtc_compute_config(struct intel_crtc *crtc,
 				     struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
 	int clock_limit = i915->max_dotclk_freq;
+	int ret;
+
+	ret = intel_crtc_compute_pipe_src(crtc_state);
+	if (ret)
+		return ret;
 
 	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
 
 	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
-	if (crtc_state->bigjoiner)
-		crtc_state->pipe_src_w /= 2;
 
 	intel_splitter_adjust_timings(crtc_state, pipe_mode);
 
@@ -2837,27 +2874,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	/*
-	 * Pipe horizontal size must be even in:
-	 * - DVO ganged mode
-	 * - LVDS dual channel mode
-	 * - Double wide pipe
-	 */
-	if (crtc_state->pipe_src_w & 1) {
-		if (crtc_state->double_wide) {
-			drm_dbg_kms(&i915->drm,
-				    "Odd pipe source width not supported with double wide pipe\n");
-			return -EINVAL;
-		}
-
-		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-		    intel_is_dual_link_lvds(i915)) {
-			drm_dbg_kms(&i915->drm,
-				    "Odd pipe source width not supported with dual link LVDS\n");
-			return -EINVAL;
-		}
-	}
-
 	intel_crtc_compute_pixel_rate(crtc_state);
 
 	if (crtc_state->has_pch_encoder)
-- 
2.34.1

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Extract intel_crtc_compute_pipe_src()

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_crtc_compute_config() doesn't really tell a unified story.
> Let's chunk it up into pieces. We'll start with
> intel_crtc_compute_pipe_src().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

with just one clarification below

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 62 ++++++++++++--------
>  1 file changed, 39 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 19417ff975c6..3d3fddd3f452 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2798,18 +2798,55 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
>  	intel_crtc_readout_derived_state(crtc_state);
>  }
>  
> +static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +
> +	if (crtc_state->bigjoiner)
> +		crtc_state->pipe_src_w /= 2;
> +
> +	/*
> +	 * Pipe horizontal size must be even in:
> +	 * - DVO ganged mode
> +	 * - LVDS dual channel mode
> +	 * - Double wide pipe
> +	 */
> +	if (crtc_state->pipe_src_w & 1) {
> +		if (crtc_state->double_wide) {
> +			drm_dbg_kms(&i915->drm,
> +				    "[CRTC:%d:%s] Odd pipe source width not supported with double wide pipe\n",
> +				    crtc->base.base.id, crtc->base.name);
> +			return -EINVAL;
> +		}
> +
> +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> +		    intel_is_dual_link_lvds(i915)) {
> +			drm_dbg_kms(&i915->drm,
> +				    "[CRTC:%d:%s] Odd pipe source width not supported with dual link LVDS\n",
> +				    crtc->base.base.id, crtc->base.name);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  				     struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	int clock_limit = i915->max_dotclk_freq;
> +	int ret;
> +
> +	ret = intel_crtc_compute_pipe_src(crtc_state);

Here crtc_state->pipe_src_w would already have been populated right?
Just wanted to double check since we are moving this earlier in the function

Manasi

> +	if (ret)
> +		return ret;
>  
>  	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
>  
>  	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> -	if (crtc_state->bigjoiner)
> -		crtc_state->pipe_src_w /= 2;
>  
>  	intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
> @@ -2837,27 +2874,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	/*
> -	 * Pipe horizontal size must be even in:
> -	 * - DVO ganged mode
> -	 * - LVDS dual channel mode
> -	 * - Double wide pipe
> -	 */
> -	if (crtc_state->pipe_src_w & 1) {
> -		if (crtc_state->double_wide) {
> -			drm_dbg_kms(&i915->drm,
> -				    "Odd pipe source width not supported with double wide pipe\n");
> -			return -EINVAL;
> -		}
> -
> -		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -		    intel_is_dual_link_lvds(i915)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "Odd pipe source width not supported with dual link LVDS\n");
> -			return -EINVAL;
> -		}
> -	}
> -
>  	intel_crtc_compute_pixel_rate(crtc_state);
>  
>  	if (crtc_state->has_pch_encoder)
> -- 
> 2.34.1
> 

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Extract intel_crtc_compute_pipe_src()

[Ville Syrjälä]

On Wed, Feb 16, 2022 at 11:35:39AM -0800, Navare, Manasi wrote:
> On Tue, Feb 15, 2022 at 08:32:02PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > intel_crtc_compute_config() doesn't really tell a unified story.
> > Let's chunk it up into pieces. We'll start with
> > intel_crtc_compute_pipe_src().
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> with just one clarification below
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 62 ++++++++++++--------
> >  1 file changed, 39 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 19417ff975c6..3d3fddd3f452 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2798,18 +2798,55 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
> >  	intel_crtc_readout_derived_state(crtc_state);
> >  }
> >  
> > +static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > +
> > +	if (crtc_state->bigjoiner)
> > +		crtc_state->pipe_src_w /= 2;
> > +
> > +	/*
> > +	 * Pipe horizontal size must be even in:
> > +	 * - DVO ganged mode
> > +	 * - LVDS dual channel mode
> > +	 * - Double wide pipe
> > +	 */
> > +	if (crtc_state->pipe_src_w & 1) {
> > +		if (crtc_state->double_wide) {
> > +			drm_dbg_kms(&i915->drm,
> > +				    "[CRTC:%d:%s] Odd pipe source width not supported with double wide pipe\n",
> > +				    crtc->base.base.id, crtc->base.name);
> > +			return -EINVAL;
> > +		}
> > +
> > +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > +		    intel_is_dual_link_lvds(i915)) {
> > +			drm_dbg_kms(&i915->drm,
> > +				    "[CRTC:%d:%s] Odd pipe source width not supported with dual link LVDS\n",
> > +				    crtc->base.base.id, crtc->base.name);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  				     struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
> >  	int clock_limit = i915->max_dotclk_freq;
> > +	int ret;
> > +
> > +	ret = intel_crtc_compute_pipe_src(crtc_state);
> 
> Here crtc_state->pipe_src_w would already have been populated right?
> Just wanted to double check since we are moving this earlier in the function

Yeah it's initially set up already in intel_modeset_pipe_config()
before even calling the encoder compute_config() hooks, and
intel_crtc_compute_config() gets called after those.

I'd actually prefer to calculate it completely here, but we
currently set up the panel fitter stuff already in the 
encoder->compute_config() hook. So the order has to as is,
for the moment at least.

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 07/12] drm/i915: Extract intel_crtc_compute_pipe_mode()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pull intel_crtc_compute_pipe_mode() out from
intel_crtc_compute_config(). Since it's semi related
we'll suck in the max dotclock/double wide checks in
as well.

And we'll pimp the debugs while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 28 ++++++++++++++------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3d3fddd3f452..6ff58164929c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2832,17 +2832,12 @@ static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
-static int intel_crtc_compute_config(struct intel_crtc *crtc,
-				     struct intel_crtc_state *crtc_state)
+static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
 	int clock_limit = i915->max_dotclk_freq;
-	int ret;
-
-	ret = intel_crtc_compute_pipe_src(crtc_state);
-	if (ret)
-		return ret;
 
 	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
 
@@ -2868,12 +2863,29 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	if (pipe_mode->crtc_clock > clock_limit) {
 		drm_dbg_kms(&i915->drm,
-			    "requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
+			    "[CRTC:%d:%s] requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
+			    crtc->base.base.id, crtc->base.name,
 			    pipe_mode->crtc_clock, clock_limit,
 			    yesno(crtc_state->double_wide));
 		return -EINVAL;
 	}
 
+	return 0;
+}
+
+static int intel_crtc_compute_config(struct intel_crtc *crtc,
+				     struct intel_crtc_state *crtc_state)
+{
+	int ret;
+
+	ret = intel_crtc_compute_pipe_src(crtc_state);
+	if (ret)
+		return ret;
+
+	ret = intel_crtc_compute_pipe_mode(crtc_state);
+	if (ret)
+		return ret;
+
 	intel_crtc_compute_pixel_rate(crtc_state);
 
 	if (crtc_state->has_pch_encoder)
-- 
2.34.1

Re: [Intel-gfx] [PATCH 07/12] drm/i915: Extract intel_crtc_compute_pipe_mode()

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:03PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull intel_crtc_compute_pipe_mode() out from
> intel_crtc_compute_config(). Since it's semi related
> we'll suck in the max dotclock/double wide checks in
> as well.
> 
> And we'll pimp the debugs while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yup looks lot more organized

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 28 ++++++++++++++------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3d3fddd3f452..6ff58164929c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2832,17 +2832,12 @@ static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> -static int intel_crtc_compute_config(struct intel_crtc *crtc,
> -				     struct intel_crtc_state *crtc_state)
> +static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	int clock_limit = i915->max_dotclk_freq;
> -	int ret;
> -
> -	ret = intel_crtc_compute_pipe_src(crtc_state);
> -	if (ret)
> -		return ret;
>  
>  	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
>  
> @@ -2868,12 +2863,29 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	if (pipe_mode->crtc_clock > clock_limit) {
>  		drm_dbg_kms(&i915->drm,
> -			    "requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> +			    "[CRTC:%d:%s] requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
> +			    crtc->base.base.id, crtc->base.name,
>  			    pipe_mode->crtc_clock, clock_limit,
>  			    yesno(crtc_state->double_wide));
>  		return -EINVAL;
>  	}
>  
> +	return 0;
> +}
> +
> +static int intel_crtc_compute_config(struct intel_crtc *crtc,
> +				     struct intel_crtc_state *crtc_state)
> +{
> +	int ret;
> +
> +	ret = intel_crtc_compute_pipe_src(crtc_state);
> +	if (ret)
> +		return ret;
> +
> +	ret = intel_crtc_compute_pipe_mode(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	intel_crtc_compute_pixel_rate(crtc_state);
>  
>  	if (crtc_state->has_pch_encoder)
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 08/12] drm/i915: Fix MSO vs. bigjoiner timings confusion

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When calculating pipe_mode and when doing readout we need
to order our steps correctly.

1. We start with adjusted_mode crtc timings being populated
   with the transcoder timings (either via readout or
   compute_config(). These will be per-segment for MSO.
2. For all other uses we want the full crtc timings so
   we ask intel_splitter_adjust_timings() to expand
   the per-segment numbers to their full glory
3. If bigjoiner is used we the divide the full numbers
   down to per-pipe numbers using intel_bigjoiner_adjust_timings()

During readout we also have to reconstruct the adjusted_mode
normal timings (ie. not the crtc_ stuff). These are supposed
to reflect the full timings of the display. So we grab these
between steps 2 and 3.

The "user" mode readout (mainly done for fastboot purposes)
should be whatever mode the user would have used had they
asked us to do a modeset. We want the full timings for this
as the per-segment timings are not suppoesed to be user visible.
Also the user mode normal timings hdisplay/vdisplay need to
match PIPESRC (that is where we get our PIPESRC size
we doing a modeset with a user supplied mode).

And we end up with
- adjusted_mode normal timigns == full timings
- adjusted_mode crtc timings == transcoder timings
  (per-segment timings for MSO, full timings otherwise)
- pipe_mode normal/crtc timings == pipe timings
  (full timings divided by the number of bigjoiner pipes, if any)
- user mode normal timings == full timings with
  hdisplay/vdisplay replaced with PIPESRC size
- user mode crtc timings == full timings

Yes, that is a lot of timings. One day we'll try to remove
some of the ones we don't actually need to keep around...

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 50 +++++++++++++-------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6ff58164929c..131be3bb8026 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2769,25 +2769,33 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
 	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 
+	/*
+	 * Start with the adjusted_mode crtc timings, which
+	 * have been filled with the transcoder timings.
+	 */
 	drm_mode_copy(pipe_mode, adjusted_mode);
 
-	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
-
-	if (crtc_state->splitter.enable) {
-		intel_splitter_adjust_timings(crtc_state, pipe_mode);
-
-		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
-		intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
-	} else {
-		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
-		intel_mode_from_crtc_timings(adjusted_mode, adjusted_mode);
-	}
-
-	intel_crtc_compute_pixel_rate(crtc_state);
-
-	drm_mode_copy(mode, adjusted_mode);
+	/* Expand MSO per-segment transcoder timings to full */
+	intel_splitter_adjust_timings(crtc_state, pipe_mode);
+
+	/*
+	 * We want the full numbers in adjusted_mode normal timings,
+	 * adjusted_mode crtc timings are left with the raw transcoder
+	 * timings.
+	 */
+	intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
+
+	/* Populate the "user" mode with full numbers */
+	drm_mode_copy(mode, pipe_mode);
+	intel_mode_from_crtc_timings(mode, mode);
 	mode->hdisplay = crtc_state->pipe_src_w << crtc_state->bigjoiner;
 	mode->vdisplay = crtc_state->pipe_src_h;
+
+	/* Derive per-pipe timings in case bigjoiner is used */
+	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
+	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
+
+	intel_crtc_compute_pixel_rate(crtc_state);
 }
 
 static void intel_encoder_get_config(struct intel_encoder *encoder,
@@ -2836,15 +2844,21 @@ static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
 	int clock_limit = i915->max_dotclk_freq;
 
-	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
-
-	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
+	/*
+	 * Start with the adjusted_mode crtc timings, which
+	 * have been filled with the transcoder timings.
+	 */
+	drm_mode_copy(pipe_mode, adjusted_mode);
 
+	/* Expand MSO per-segment transcoder timings to full */
 	intel_splitter_adjust_timings(crtc_state, pipe_mode);
 
+	/* Derive per-pipe timings in case bigjoiner is used */
+	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
 	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
 
 	if (DISPLAY_VER(i915) < 4) {
-- 
2.34.1

Re: [Intel-gfx] [PATCH 08/12] drm/i915: Fix MSO vs. bigjoiner timings confusion

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:04PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When calculating pipe_mode and when doing readout we need
> to order our steps correctly.
> 
> 1. We start with adjusted_mode crtc timings being populated
>    with the transcoder timings (either via readout or
>    compute_config(). These will be per-segment for MSO.
> 2. For all other uses we want the full crtc timings so
>    we ask intel_splitter_adjust_timings() to expand
>    the per-segment numbers to their full glory
> 3. If bigjoiner is used we the divide the full numbers
>    down to per-pipe numbers using intel_bigjoiner_adjust_timings()
> 
> During readout we also have to reconstruct the adjusted_mode
> normal timings (ie. not the crtc_ stuff). These are supposed
> to reflect the full timings of the display. So we grab these
> between steps 2 and 3.
> 
> The "user" mode readout (mainly done for fastboot purposes)
> should be whatever mode the user would have used had they
> asked us to do a modeset. We want the full timings for this
> as the per-segment timings are not suppoesed to be user visible.
> Also the user mode normal timings hdisplay/vdisplay need to
> match PIPESRC (that is where we get our PIPESRC size
> we doing a modeset with a user supplied mode).
> 
> And we end up with
> - adjusted_mode normal timigns == full timings
> - adjusted_mode crtc timings == transcoder timings
>   (per-segment timings for MSO, full timings otherwise)
> - pipe_mode normal/crtc timings == pipe timings
>   (full timings divided by the number of bigjoiner pipes, if any)
> - user mode normal timings == full timings with
>   hdisplay/vdisplay replaced with PIPESRC size
> - user mode crtc timings == full timings
> 
> Yes, that is a lot of timings. One day we'll try to remove
> some of the ones we don't actually need to keep around...
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 50 +++++++++++++-------
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6ff58164929c..131be3bb8026 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2769,25 +2769,33 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  
> +	/*
> +	 * Start with the adjusted_mode crtc timings, which
> +	 * have been filled with the transcoder timings.
> +	 */
>  	drm_mode_copy(pipe_mode, adjusted_mode);
>  
> -	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> -
> -	if (crtc_state->splitter.enable) {
> -		intel_splitter_adjust_timings(crtc_state, pipe_mode);
> -
> -		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> -		intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
> -	} else {
> -		intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> -		intel_mode_from_crtc_timings(adjusted_mode, adjusted_mode);
> -	}
> -
> -	intel_crtc_compute_pixel_rate(crtc_state);
> -
> -	drm_mode_copy(mode, adjusted_mode);
> +	/* Expand MSO per-segment transcoder timings to full */
> +	intel_splitter_adjust_timings(crtc_state, pipe_mode);
> +
> +	/*
> +	 * We want the full numbers in adjusted_mode normal timings,
> +	 * adjusted_mode crtc timings are left with the raw transcoder
> +	 * timings.
> +	 */
> +	intel_mode_from_crtc_timings(adjusted_mode, pipe_mode);
> +
> +	/* Populate the "user" mode with full numbers */
> +	drm_mode_copy(mode, pipe_mode);
> +	intel_mode_from_crtc_timings(mode, mode);
>  	mode->hdisplay = crtc_state->pipe_src_w << crtc_state->bigjoiner;
>  	mode->vdisplay = crtc_state->pipe_src_h;
> +
> +	/* Derive per-pipe timings in case bigjoiner is used */
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> +	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
> +
> +	intel_crtc_compute_pixel_rate(crtc_state);
>  }
>  
>  static void intel_encoder_get_config(struct intel_encoder *encoder,
> @@ -2836,15 +2844,21 @@ static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
>  	int clock_limit = i915->max_dotclk_freq;
>  
> -	drm_mode_copy(pipe_mode, &crtc_state->hw.adjusted_mode);
> -
> -	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
> +	/*
> +	 * Start with the adjusted_mode crtc timings, which
> +	 * have been filled with the transcoder timings.
> +	 */
> +	drm_mode_copy(pipe_mode, adjusted_mode);
>  
> +	/* Expand MSO per-segment transcoder timings to full */
>  	intel_splitter_adjust_timings(crtc_state, pipe_mode);
>  
> +	/* Derive per-pipe timings in case bigjoiner is used */
> +	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
>  	intel_mode_from_crtc_timings(pipe_mode, pipe_mode);
>  
>  	if (DISPLAY_VER(i915) < 4) {
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 09/12] drm/i915: Start tracking PIPESRC as a drm_rect

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Instead of just having the pipe_src_{w,h} let's use a full
drm_rect for it. This will be particularly useful to astract
away some bigjoiner details.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c | 15 ++--
 drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 56 ++++++++++-----
 .../drm/i915/display/intel_display_debugfs.c  |  4 +-
 .../drm/i915/display/intel_display_types.h    |  2 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  | 12 ++--
 drivers/gpu/drm/i915/display/intel_panel.c    | 70 +++++++++----------
 drivers/gpu/drm/i915/display/skl_scaler.c     | 12 ++--
 .../drm/i915/display/skl_universal_plane.c    |  2 +-
 9 files changed, 97 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 0e15fe908855..4f5a17e008a5 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -607,8 +607,8 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 	struct drm_framebuffer *fb = plane_state->hw.fb;
 	struct drm_rect *src = &plane_state->uapi.src;
 	struct drm_rect *dst = &plane_state->uapi.dst;
+	const struct drm_rect *clip = &crtc_state->pipe_src;
 	unsigned int rotation = plane_state->hw.rotation;
-	struct drm_rect clip = {};
 	int hscale, vscale;
 
 	if (!fb) {
@@ -628,28 +628,23 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 		return -ERANGE;
 	}
 
-	if (crtc_state->hw.enable) {
-		clip.x2 = crtc_state->pipe_src_w;
-		clip.y2 = crtc_state->pipe_src_h;
-	}
-
 	/* right side of the image is on the slave crtc, adjust dst to match */
 	if (intel_crtc_is_bigjoiner_slave(crtc_state))
-		drm_rect_translate(dst, -crtc_state->pipe_src_w, 0);
+		drm_rect_translate(dst, -drm_rect_width(&crtc_state->pipe_src), 0);
 
 	/*
 	 * FIXME: This might need further adjustment for seamless scaling
 	 * with phase information, for the 2p2 and 2p1 scenarios.
 	 */
-	plane_state->uapi.visible = drm_rect_clip_scaled(src, dst, &clip);
+	plane_state->uapi.visible = drm_rect_clip_scaled(src, dst, clip);
 
 	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
 
 	if (!can_position && plane_state->uapi.visible &&
-	    !drm_rect_equals(dst, &clip)) {
+	    !drm_rect_equals(dst, clip)) {
 		drm_dbg_kms(&i915->drm, "Plane must cover entire CRTC\n");
 		drm_rect_debug_print("dst: ", dst, false);
-		drm_rect_debug_print("clip: ", &clip, false);
+		drm_rect_debug_print("clip: ", clip, false);
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 3e80763aa828..1f448f4e9aaf 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -154,7 +154,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	plane_state->uapi.dst = dst;
 	if (intel_crtc_is_bigjoiner_slave(crtc_state))
 		drm_rect_translate(&plane_state->uapi.dst,
-				   -crtc_state->pipe_src_w, 0);
+				   -drm_rect_width(&crtc_state->pipe_src), 0);
 
 	ret = intel_cursor_check_surface(plane_state);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 131be3bb8026..47b5d8cc16fd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2683,8 +2683,8 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
 		return pixel_rate;
 
 	drm_rect_init(&src, 0, 0,
-		      crtc_state->pipe_src_w << 16,
-		      crtc_state->pipe_src_h << 16);
+		      drm_rect_width(&crtc_state->pipe_src) << 16,
+		      drm_rect_height(&crtc_state->pipe_src) << 16);
 
 	return intel_adjusted_rate(&src, &crtc_state->pch_pfit.dst,
 				   pixel_rate);
@@ -2788,8 +2788,9 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
 	/* Populate the "user" mode with full numbers */
 	drm_mode_copy(mode, pipe_mode);
 	intel_mode_from_crtc_timings(mode, mode);
-	mode->hdisplay = crtc_state->pipe_src_w << crtc_state->bigjoiner;
-	mode->vdisplay = crtc_state->pipe_src_h;
+	mode->hdisplay = drm_rect_width(&crtc_state->pipe_src) *
+		(hweight8(crtc_state->bigjoiner_pipes) ?: 1);
+	mode->vdisplay = drm_rect_height(&crtc_state->pipe_src);
 
 	/* Derive per-pipe timings in case bigjoiner is used */
 	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);
@@ -2806,13 +2807,26 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
 	intel_crtc_readout_derived_state(crtc_state);
 }
 
+static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
+{
+	int width, height;
+
+	if (!crtc_state->bigjoiner)
+		return;
+
+	width = drm_rect_width(&crtc_state->pipe_src);
+	height = drm_rect_height(&crtc_state->pipe_src);
+
+	drm_rect_init(&crtc_state->pipe_src, 0, 0,
+		      width / 2, height);
+}
+
 static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 
-	if (crtc_state->bigjoiner)
-		crtc_state->pipe_src_w /= 2;
+	intel_bigjoiner_compute_pipe_src(crtc_state);
 
 	/*
 	 * Pipe horizontal size must be even in:
@@ -2820,7 +2834,7 @@ static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
 	 * - LVDS dual channel mode
 	 * - Double wide pipe
 	 */
-	if (crtc_state->pipe_src_w & 1) {
+	if (drm_rect_width(&crtc_state->pipe_src) & 1) {
 		if (crtc_state->double_wide) {
 			drm_dbg_kms(&i915->drm,
 				    "[CRTC:%d:%s] Odd pipe source width not supported with double wide pipe\n",
@@ -3107,14 +3121,15 @@ static void intel_set_pipe_src_size(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);
+	int width = drm_rect_width(&crtc_state->pipe_src);
+	int height = drm_rect_height(&crtc_state->pipe_src);
 	enum pipe pipe = crtc->pipe;
 
 	/* pipesrc controls the size that is scaled from, which should
 	 * always be the user's requested size.
 	 */
 	intel_de_write(dev_priv, PIPESRC(pipe),
-		       PIPESRC_WIDTH(crtc_state->pipe_src_w - 1) |
-		       PIPESRC_HEIGHT(crtc_state->pipe_src_h - 1));
+		       PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height - 1));
 }
 
 static bool intel_pipe_is_interlaced(const struct intel_crtc_state *crtc_state)
@@ -3185,8 +3200,10 @@ static void intel_get_pipe_src_size(struct intel_crtc *crtc,
 	u32 tmp;
 
 	tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
-	pipe_config->pipe_src_w = REG_FIELD_GET(PIPESRC_WIDTH_MASK, tmp) + 1;
-	pipe_config->pipe_src_h = REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1;
+
+	drm_rect_init(&pipe_config->pipe_src, 0, 0,
+		      REG_FIELD_GET(PIPESRC_WIDTH_MASK, tmp) + 1,
+		      REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1);
 }
 
 static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
@@ -5572,9 +5589,8 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 	drm_mode_debug_printmodeline(&pipe_config->hw.pipe_mode);
 	intel_dump_crtc_timings(dev_priv, &pipe_config->hw.pipe_mode);
 	drm_dbg_kms(&dev_priv->drm,
-		    "port clock: %d, pipe src size: %dx%d, pixel rate %d\n",
-		    pipe_config->port_clock,
-		    pipe_config->pipe_src_w, pipe_config->pipe_src_h,
+		    "port clock: %d, pipe src: " DRM_RECT_FMT ", pixel rate %d\n",
+		    pipe_config->port_clock, DRM_RECT_ARG(&pipe_config->pipe_src),
 		    pipe_config->pixel_rate);
 
 	drm_dbg_kms(&dev_priv->drm, "linetime: %d, ips linetime: %d\n",
@@ -5869,6 +5885,7 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
 	struct drm_i915_private *i915 = to_i915(pipe_config->uapi.crtc->dev);
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
+	int pipe_src_w, pipe_src_h;
 	int base_bpp, ret, i;
 	bool retry = true;
 
@@ -5904,8 +5921,9 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
 	 * can be changed by the connectors in the below retry loop.
 	 */
 	drm_mode_get_hv_timing(&pipe_config->hw.mode,
-			       &pipe_config->pipe_src_w,
-			       &pipe_config->pipe_src_h);
+			       &pipe_src_w, &pipe_src_h);
+	drm_rect_init(&pipe_config->pipe_src, 0, 0,
+		      pipe_src_w, pipe_src_h);
 
 	for_each_new_connector_in_state(&state->base, connector, connector_state, i) {
 		struct intel_encoder *encoder =
@@ -6482,8 +6500,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_BOOL(pch_pfit.force_thru);
 
 	if (!fastset) {
-		PIPE_CONF_CHECK_I(pipe_src_w);
-		PIPE_CONF_CHECK_I(pipe_src_h);
+		PIPE_CONF_CHECK_I(pipe_src.x1);
+		PIPE_CONF_CHECK_I(pipe_src.y1);
+		PIPE_CONF_CHECK_I(pipe_src.x2);
+		PIPE_CONF_CHECK_I(pipe_src.y2);
 
 		PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
 		if (current_config->pch_pfit.enabled) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 1a202a5c39a5..475ae7e7f760 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -928,8 +928,8 @@ static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
 			   yesno(crtc_state->hw.active),
 			   DRM_MODE_ARG(&crtc_state->hw.adjusted_mode));
 
-		seq_printf(m, "\tpipe src size=%dx%d, dither=%s, bpp=%d\n",
-			   crtc_state->pipe_src_w, crtc_state->pipe_src_h,
+		seq_printf(m, "\tpipe src=" DRM_RECT_FMT ", dither=%s, bpp=%d\n",
+			   DRM_RECT_ARG(&crtc_state->pipe_src),
 			   yesno(crtc_state->dither), crtc_state->pipe_bpp);
 
 		intel_scaler_info(m, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 641ecae42198..c1a291b6b638 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -975,7 +975,7 @@ struct intel_crtc_state {
 	/* Pipe source size (ie. panel fitter input size)
 	 * All planes will be positioned inside this space,
 	 * and get clipped at the edges. */
-	int pipe_src_w, pipe_src_h;
+	struct drm_rect pipe_src;
 
 	/*
 	 * Pipe pixel rate, adjusted for
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 76845d34ad0c..631e1f1dc5e6 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -960,14 +960,16 @@ static int check_overlay_dst(struct intel_overlay *overlay,
 {
 	const struct intel_crtc_state *pipe_config =
 		overlay->crtc->config;
+	int pipe_src_w = drm_rect_width(&pipe_config->pipe_src);
+	int pipe_src_h = drm_rect_height(&pipe_config->pipe_src);
 
 	if (rec->dst_height == 0 || rec->dst_width == 0)
 		return -EINVAL;
 
-	if (rec->dst_x < pipe_config->pipe_src_w &&
-	    rec->dst_x + rec->dst_width <= pipe_config->pipe_src_w &&
-	    rec->dst_y < pipe_config->pipe_src_h &&
-	    rec->dst_y + rec->dst_height <= pipe_config->pipe_src_h)
+	if (rec->dst_x < pipe_src_w &&
+	    rec->dst_x + rec->dst_width <= pipe_src_w &&
+	    rec->dst_y < pipe_src_h &&
+	    rec->dst_y + rec->dst_height <= pipe_src_h)
 		return 0;
 	else
 		return -EINVAL;
@@ -1160,7 +1162,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 		crtc->overlay = overlay;
 
 		/* line too wide, i.e. one-line-mode */
-		if (crtc->config->pipe_src_w > 1024 &&
+		if (drm_rect_width(&crtc->config->pipe_src) > 1024 &&
 		    crtc->config->gmch_pfit.control & PFIT_ENABLE) {
 			overlay->pfit_active = true;
 			update_pfit_vscale_ratio(overlay);
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index a0c8e43db5eb..6cd6d4fdd5ad 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -205,18 +205,20 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
 {
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
+	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
+	int pipe_src_h = drm_rect_height(&crtc_state->pipe_src);
 	int x, y, width, height;
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
-	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h &&
+	if (adjusted_mode->crtc_hdisplay == pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_src_h &&
 	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		return 0;
 
 	switch (conn_state->scaling_mode) {
 	case DRM_MODE_SCALE_CENTER:
-		width = crtc_state->pipe_src_w;
-		height = crtc_state->pipe_src_h;
+		width = pipe_src_w;
+		height = pipe_src_h;
 		x = (adjusted_mode->crtc_hdisplay - width + 1)/2;
 		y = (adjusted_mode->crtc_vdisplay - height + 1)/2;
 		break;
@@ -224,19 +226,17 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
 		{
-			u32 scaled_width = adjusted_mode->crtc_hdisplay
-				* crtc_state->pipe_src_h;
-			u32 scaled_height = crtc_state->pipe_src_w
-				* adjusted_mode->crtc_vdisplay;
+			u32 scaled_width = adjusted_mode->crtc_hdisplay * pipe_src_h;
+			u32 scaled_height = pipe_src_w * adjusted_mode->crtc_vdisplay;
 			if (scaled_width > scaled_height) { /* pillar */
-				width = scaled_height / crtc_state->pipe_src_h;
+				width = scaled_height / pipe_src_h;
 				if (width & 1)
 					width++;
 				x = (adjusted_mode->crtc_hdisplay - width + 1) / 2;
 				y = 0;
 				height = adjusted_mode->crtc_vdisplay;
 			} else if (scaled_width < scaled_height) { /* letter */
-				height = scaled_width / crtc_state->pipe_src_w;
+				height = scaled_width / pipe_src_w;
 				if (height & 1)
 				    height++;
 				y = (adjusted_mode->crtc_vdisplay - height + 1) / 2;
@@ -251,8 +251,8 @@ static int pch_panel_fitting(struct intel_crtc_state *crtc_state,
 		break;
 
 	case DRM_MODE_SCALE_NONE:
-		WARN_ON(adjusted_mode->crtc_hdisplay != crtc_state->pipe_src_w);
-		WARN_ON(adjusted_mode->crtc_vdisplay != crtc_state->pipe_src_h);
+		WARN_ON(adjusted_mode->crtc_hdisplay != pipe_src_w);
+		WARN_ON(adjusted_mode->crtc_vdisplay != pipe_src_h);
 		fallthrough;
 	case DRM_MODE_SCALE_FULLSCREEN:
 		x = y = 0;
@@ -333,10 +333,10 @@ static void i965_scale_aspect(struct intel_crtc_state *crtc_state,
 {
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
-	u32 scaled_width = adjusted_mode->crtc_hdisplay *
-		crtc_state->pipe_src_h;
-	u32 scaled_height = crtc_state->pipe_src_w *
-		adjusted_mode->crtc_vdisplay;
+	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
+	int pipe_src_h = drm_rect_height(&crtc_state->pipe_src);
+	u32 scaled_width = adjusted_mode->crtc_hdisplay * pipe_src_h;
+	u32 scaled_height = pipe_src_w * adjusted_mode->crtc_vdisplay;
 
 	/* 965+ is easy, it does everything in hw */
 	if (scaled_width > scaled_height)
@@ -345,7 +345,7 @@ static void i965_scale_aspect(struct intel_crtc_state *crtc_state,
 	else if (scaled_width < scaled_height)
 		*pfit_control |= PFIT_ENABLE |
 			PFIT_SCALING_LETTER;
-	else if (adjusted_mode->crtc_hdisplay != crtc_state->pipe_src_w)
+	else if (adjusted_mode->crtc_hdisplay != pipe_src_w)
 		*pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
 }
 
@@ -354,10 +354,10 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 			      u32 *border)
 {
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
-	u32 scaled_width = adjusted_mode->crtc_hdisplay *
-		crtc_state->pipe_src_h;
-	u32 scaled_height = crtc_state->pipe_src_w *
-		adjusted_mode->crtc_vdisplay;
+	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
+	int pipe_src_h = drm_rect_height(&crtc_state->pipe_src);
+	u32 scaled_width = adjusted_mode->crtc_hdisplay * pipe_src_h;
+	u32 scaled_height = pipe_src_w * adjusted_mode->crtc_vdisplay;
 	u32 bits;
 
 	/*
@@ -367,12 +367,11 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 	 */
 	if (scaled_width > scaled_height) { /* pillar */
 		centre_horizontally(adjusted_mode,
-				    scaled_height /
-				    crtc_state->pipe_src_h);
+				    scaled_height / pipe_src_h);
 
 		*border = LVDS_BORDER_ENABLE;
-		if (crtc_state->pipe_src_h != adjusted_mode->crtc_vdisplay) {
-			bits = panel_fitter_scaling(crtc_state->pipe_src_h,
+		if (pipe_src_h != adjusted_mode->crtc_vdisplay) {
+			bits = panel_fitter_scaling(pipe_src_h,
 						    adjusted_mode->crtc_vdisplay);
 
 			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
@@ -383,12 +382,11 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 		}
 	} else if (scaled_width < scaled_height) { /* letter */
 		centre_vertically(adjusted_mode,
-				  scaled_width /
-				  crtc_state->pipe_src_w);
+				  scaled_width / pipe_src_w);
 
 		*border = LVDS_BORDER_ENABLE;
-		if (crtc_state->pipe_src_w != adjusted_mode->crtc_hdisplay) {
-			bits = panel_fitter_scaling(crtc_state->pipe_src_w,
+		if (pipe_src_w != adjusted_mode->crtc_hdisplay) {
+			bits = panel_fitter_scaling(pipe_src_w,
 						    adjusted_mode->crtc_hdisplay);
 
 			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
@@ -413,10 +411,12 @@ static int gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
+	int pipe_src_h = drm_rect_height(&crtc_state->pipe_src);
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
-	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h)
+	if (adjusted_mode->crtc_hdisplay == pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_src_h)
 		goto out;
 
 	switch (conn_state->scaling_mode) {
@@ -425,8 +425,8 @@ static int gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 		 * For centered modes, we have to calculate border widths &
 		 * heights and modify the values programmed into the CRTC.
 		 */
-		centre_horizontally(adjusted_mode, crtc_state->pipe_src_w);
-		centre_vertically(adjusted_mode, crtc_state->pipe_src_h);
+		centre_horizontally(adjusted_mode, pipe_src_w);
+		centre_vertically(adjusted_mode, pipe_src_h);
 		border = LVDS_BORDER_ENABLE;
 		break;
 	case DRM_MODE_SCALE_ASPECT:
@@ -442,8 +442,8 @@ static int gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 		 * Full scaling, even if it changes the aspect ratio.
 		 * Fortunately this is all done for us in hw.
 		 */
-		if (crtc_state->pipe_src_h != adjusted_mode->crtc_vdisplay ||
-		    crtc_state->pipe_src_w != adjusted_mode->crtc_hdisplay) {
+		if (pipe_src_h != adjusted_mode->crtc_vdisplay ||
+		    pipe_src_w != adjusted_mode->crtc_hdisplay) {
 			pfit_control |= PFIT_ENABLE;
 			if (DISPLAY_VER(dev_priv) >= 4)
 				pfit_control |= PFIT_SCALING_AUTO;
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index c2e94118566b..998128bac8c0 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -197,7 +197,8 @@ int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state)
 	return skl_update_scaler(crtc_state, !crtc_state->hw.active,
 				 SKL_CRTC_INDEX,
 				 &crtc_state->scaler_state.scaler_id,
-				 crtc_state->pipe_src_w, crtc_state->pipe_src_h,
+				 drm_rect_width(&crtc_state->pipe_src),
+				 drm_rect_height(&crtc_state->pipe_src),
 				 width, height, NULL, 0,
 				 crtc_state->pch_pfit.enabled);
 }
@@ -400,10 +401,6 @@ void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
-	struct drm_rect src = {
-		.x2 = crtc_state->pipe_src_w << 16,
-		.y2 = crtc_state->pipe_src_h << 16,
-	};
 	const struct drm_rect *dst = &crtc_state->pch_pfit.dst;
 	u16 uv_rgb_hphase, uv_rgb_vphase;
 	enum pipe pipe = crtc->pipe;
@@ -413,6 +410,7 @@ void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	int y = dst->y1;
 	int hscale, vscale;
 	unsigned long irqflags;
+	struct drm_rect src;
 	int id;
 	u32 ps_ctrl;
 
@@ -423,6 +421,10 @@ void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 			crtc_state->scaler_state.scaler_id < 0))
 		return;
 
+	drm_rect_init(&src, 0, 0,
+		      drm_rect_width(&crtc_state->pipe_src) << 16,
+		      drm_rect_height(&crtc_state->pipe_src) << 16);
+
 	hscale = drm_rect_calc_hscale(&src, dst, 0, INT_MAX);
 	vscale = drm_rect_calc_vscale(&src, dst, 0, INT_MAX);
 
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 1223075595ff..c73758d18b6f 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1325,7 +1325,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
 		to_i915(plane_state->uapi.plane->dev);
 	int crtc_x = plane_state->uapi.dst.x1;
 	int crtc_w = drm_rect_width(&plane_state->uapi.dst);
-	int pipe_src_w = crtc_state->pipe_src_w;
+	int pipe_src_w = drm_rect_width(&crtc_state->pipe_src);
 
 	/*
 	 * Display WA #1175: glk
-- 
2.34.1

Re: [Intel-gfx] [PATCH 09/12] drm/i915: Start tracking PIPESRC as a drm_rect

[Ville Syrjälä]

On Tue, Feb 15, 2022 at 08:32:05PM +0200, Ville Syrjala wrote:
> @@ -2788,8 +2788,9 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  	/* Populate the "user" mode with full numbers */
>  	drm_mode_copy(mode, pipe_mode);
>  	intel_mode_from_crtc_timings(mode, mode);
> -	mode->hdisplay = crtc_state->pipe_src_w << crtc_state->bigjoiner;
> -	mode->vdisplay = crtc_state->pipe_src_h;
> +	mode->hdisplay = drm_rect_width(&crtc_state->pipe_src) *
> +		(hweight8(crtc_state->bigjoiner_pipes) ?: 1);

That hweight() stuff was supposed to be in one of the later patches btw.
Looks like I accidentally squashed it here when splitting/reordering stuff.

> +	mode->vdisplay = drm_rect_height(&crtc_state->pipe_src);
>  
>  	/* Derive per-pipe timings in case bigjoiner is used */
>  	intel_bigjoiner_adjust_timings(crtc_state, pipe_mode);

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 10/12] drm/i915: Eliminate bigjoiner boolean

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since we now have the bigjoiner_pipes bitmask the boolean
is redundant. Get rid of it.

Also, populating bigjoiner_pipes already during
encoder->compute_config() allows us to use it much earlier
during the state calculation as well. The initial aim is
to use it in intel_crtc_compute_config().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 50 ++++++++-----------
 .../drm/i915/display/intel_display_debugfs.c  |  2 +-
 .../drm/i915/display/intel_display_types.h    |  3 --
 drivers/gpu/drm/i915/display/intel_dp.c       | 13 ++---
 drivers/gpu/drm/i915/display/intel_vdsc.c     |  8 +--
 .../drm/i915/display/skl_universal_plane.c    |  2 +-
 7 files changed, 36 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 1f448f4e9aaf..da6cf0515164 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -640,7 +640,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 	 * FIXME bigjoiner fastpath would be good
 	 */
 	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
-	    crtc_state->update_pipe || crtc_state->bigjoiner)
+	    crtc_state->update_pipe || crtc_state->bigjoiner_pipes)
 		goto slow;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 47b5d8cc16fd..192474163edb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1926,7 +1926,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
 		return;
 
-	if (!new_crtc_state->bigjoiner) {
+	if (!new_crtc_state->bigjoiner_pipes) {
 		intel_encoders_pre_pll_enable(state, crtc);
 
 		if (new_crtc_state->shared_dpll)
@@ -2727,7 +2727,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
 static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
 					   struct drm_display_mode *mode)
 {
-	if (!crtc_state->bigjoiner)
+	if (!crtc_state->bigjoiner_pipes)
 		return;
 
 	mode->crtc_clock /= 2;
@@ -2811,7 +2811,7 @@ static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state
 {
 	int width, height;
 
-	if (!crtc_state->bigjoiner)
+	if (!crtc_state->bigjoiner_pipes)
 		return;
 
 	width = drm_rect_width(&crtc_state->pipe_src);
@@ -4218,7 +4218,6 @@ static void intel_bigjoiner_get_config(struct intel_crtc_state *crtc_state)
 	if (((master_pipes | slave_pipes) & BIT(pipe)) == 0)
 		return;
 
-	crtc_state->bigjoiner = true;
 	crtc_state->bigjoiner_pipes =
 		BIT(get_bigjoiner_master_pipe(pipe, master_pipes, slave_pipes)) |
 		get_bigjoiner_slave_pipes(pipe, master_pipes, slave_pipes);
@@ -5804,6 +5803,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, master_crtc);
 	struct intel_crtc_state *saved_state;
 
+	WARN_ON(master_crtc_state->bigjoiner_pipes !=
+		slave_crtc_state->bigjoiner_pipes);
+
 	saved_state = kmemdup(master_crtc_state, sizeof(*saved_state), GFP_KERNEL);
 	if (!saved_state)
 		return -ENOMEM;
@@ -5834,6 +5836,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
 	slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
 	slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
 
+	WARN_ON(master_crtc_state->bigjoiner_pipes !=
+		slave_crtc_state->bigjoiner_pipes);
+
 	return 0;
 }
 
@@ -6604,7 +6609,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
 	PIPE_CONF_CHECK_I(master_transcoder);
-	PIPE_CONF_CHECK_BOOL(bigjoiner);
 	PIPE_CONF_CHECK_X(bigjoiner_pipes);
 
 	PIPE_CONF_CHECK_I(dsc.compression_enable);
@@ -7535,32 +7539,26 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
 	struct intel_crtc_state *master_crtc_state =
 		intel_atomic_get_new_crtc_state(state, master_crtc);
 	struct intel_crtc *slave_crtc;
-	u8 slave_pipes;
 
-	/*
-	 * TODO: encoder.compute_config() may be the best
-	 * place to populate the bitmask for the master crtc.
-	 * For now encoder.compute_config() just flags things
-	 * as needing bigjoiner and we populate the bitmask
-	 * here.
-	 */
-	WARN_ON(master_crtc_state->bigjoiner_pipes);
-
-	if (!master_crtc_state->bigjoiner)
+	if (!master_crtc_state->bigjoiner_pipes)
 		return 0;
 
-	slave_pipes = BIT(master_crtc->pipe + 1);
+	/* sanity check */
+	if (drm_WARN_ON(&i915->drm,
+			master_crtc->pipe != bigjoiner_master_pipe(master_crtc_state)))
+		return -EINVAL;
 
-	if (slave_pipes & ~bigjoiner_pipes(i915)) {
+	if (master_crtc_state->bigjoiner_pipes & ~bigjoiner_pipes(i915)) {
 		drm_dbg_kms(&i915->drm,
 			    "[CRTC:%d:%s] Cannot act as big joiner master "
-			    "(need 0x%x as slave pipes, only 0x%x possible)\n",
+			    "(need 0x%x as pipes, only 0x%x possible)\n",
 			    master_crtc->base.base.id, master_crtc->base.name,
-			    slave_pipes, bigjoiner_pipes(i915));
+			    master_crtc_state->bigjoiner_pipes, bigjoiner_pipes(i915));
 		return -EINVAL;
 	}
 
-	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc, slave_pipes) {
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
+					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
 		struct intel_crtc_state *slave_crtc_state;
 		int ret;
 
@@ -7594,10 +7592,8 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
 			    slave_crtc->base.base.id, slave_crtc->base.name,
 			    master_crtc->base.base.id, master_crtc->base.name);
 
-		master_crtc_state->bigjoiner_pipes =
-			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
 		slave_crtc_state->bigjoiner_pipes =
-			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
+			master_crtc_state->bigjoiner_pipes;
 
 		ret = copy_bigjoiner_crtc_state_modeset(state, slave_crtc);
 		if (ret)
@@ -7620,13 +7616,11 @@ static void kill_bigjoiner_slave(struct intel_atomic_state *state,
 		struct intel_crtc_state *slave_crtc_state =
 			intel_atomic_get_new_crtc_state(state, slave_crtc);
 
-		slave_crtc_state->bigjoiner = false;
 		slave_crtc_state->bigjoiner_pipes = 0;
 
 		intel_crtc_copy_uapi_to_hw_state_modeset(state, slave_crtc);
 	}
 
-	master_crtc_state->bigjoiner = false;
 	master_crtc_state->bigjoiner_pipes = 0;
 }
 
@@ -7936,7 +7930,7 @@ static int intel_atomic_check(struct drm_device *dev,
 			}
 		}
 
-		if (new_crtc_state->bigjoiner) {
+		if (new_crtc_state->bigjoiner_pipes) {
 			if (intel_pipes_need_modeset(state, new_crtc_state->bigjoiner_pipes)) {
 				new_crtc_state->uapi.mode_changed = true;
 				new_crtc_state->update_pipe = false;
@@ -10338,7 +10332,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			intel_encoder_get_config(encoder, crtc_state);
 
 			/* read out to slave crtc as well for bigjoiner */
-			if (crtc_state->bigjoiner) {
+			if (crtc_state->bigjoiner_pipes) {
 				struct intel_crtc *slave_crtc;
 
 				/* encoder should read be linked to bigjoiner master */
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 475ae7e7f760..c87718ae2c46 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -935,7 +935,7 @@ static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
 		intel_scaler_info(m, crtc);
 	}
 
-	if (crtc_state->bigjoiner)
+	if (crtc_state->bigjoiner_pipes)
 		seq_printf(m, "\tLinked to 0x%x pipes as a %s\n",
 			   crtc_state->bigjoiner_pipes,
 			   intel_crtc_is_bigjoiner_slave(crtc_state) ? "slave" : "master");
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c1a291b6b638..92357fdbd0f0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1199,9 +1199,6 @@ struct intel_crtc_state {
 	/* enable pipe csc? */
 	bool csc_enable;
 
-	/* enable pipe big joiner? */
-	bool bigjoiner;
-
 	/* big joiner pipe bitmask */
 	u8 bigjoiner_pipes;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1046e7fe310a..05e1da3c43e2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1424,13 +1424,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 						    pipe_config->lane_count,
 						    adjusted_mode->crtc_clock,
 						    adjusted_mode->crtc_hdisplay,
-						    pipe_config->bigjoiner,
+						    pipe_config->bigjoiner_pipes,
 						    pipe_bpp);
 		dsc_dp_slice_count =
 			intel_dp_dsc_get_slice_count(intel_dp,
 						     adjusted_mode->crtc_clock,
 						     adjusted_mode->crtc_hdisplay,
-						     pipe_config->bigjoiner);
+						     pipe_config->bigjoiner_pipes);
 		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
 			drm_dbg_kms(&dev_priv->drm,
 				    "Compressed BPP/Slice Count not supported\n");
@@ -1464,7 +1464,7 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	 * then we need to use 2 VDSC instances.
 	 */
 	if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq ||
-	    pipe_config->bigjoiner) {
+	    pipe_config->bigjoiner_pipes) {
 		if (pipe_config->dsc.slice_count < 2) {
 			drm_dbg_kms(&dev_priv->drm,
 				    "Cannot split stream to use 2 VDSC instances\n");
@@ -1500,6 +1500,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	const struct drm_display_mode *adjusted_mode =
 		&pipe_config->hw.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
@@ -1537,7 +1538,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 
 	if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay,
 				    adjusted_mode->crtc_clock))
-		pipe_config->bigjoiner = true;
+		pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
 	/*
 	 * Optimize for slow and wide for everything, because there are some
@@ -1550,8 +1551,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	 * onwards pipe joiner can be enabled without compression.
 	 */
 	drm_dbg_kms(&i915->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
-	if (ret || intel_dp->force_dsc_en || (DISPLAY_VER(i915) < 13 &&
-					      pipe_config->bigjoiner)) {
+	if (ret || intel_dp->force_dsc_en ||
+	    (DISPLAY_VER(i915) < 13 && pipe_config->bigjoiner_pipes)) {
 		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
 						  conn_state, &limits);
 		if (ret < 0)
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 545eff5bf158..28a1c982750e 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -579,7 +579,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
 	u8 num_vdsc_instances = (crtc_state->dsc.dsc_split) ? 2 : 1;
 	int i = 0;
 
-	if (crtc_state->bigjoiner)
+	if (crtc_state->bigjoiner_pipes)
 		num_vdsc_instances *= 2;
 
 	/* Populate PICTURE_PARAMETER_SET_0 registers */
@@ -1113,7 +1113,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 dss_ctl1_val = 0;
 
-	if (crtc_state->bigjoiner && !crtc_state->dsc.compression_enable) {
+	if (crtc_state->bigjoiner_pipes && !crtc_state->dsc.compression_enable) {
 		if (intel_crtc_is_bigjoiner_slave(crtc_state))
 			dss_ctl1_val |= UNCOMPRESSED_JOINER_SLAVE;
 		else
@@ -1140,7 +1140,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
 		dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
 		dss_ctl1_val |= JOINER_ENABLE;
 	}
-	if (crtc_state->bigjoiner) {
+	if (crtc_state->bigjoiner_pipes) {
 		dss_ctl1_val |= BIG_JOINER_ENABLE;
 		if (!intel_crtc_is_bigjoiner_slave(crtc_state))
 			dss_ctl1_val |= MASTER_BIG_JOINER_ENABLE;
@@ -1156,7 +1156,7 @@ void intel_dsc_disable(const struct intel_crtc_state *old_crtc_state)
 
 	/* Disable only if either of them is enabled */
 	if (old_crtc_state->dsc.compression_enable ||
-	    old_crtc_state->bigjoiner) {
+	    old_crtc_state->bigjoiner_pipes) {
 		intel_de_write(dev_priv, dss_ctl1_reg(crtc, old_crtc_state->cpu_transcoder), 0);
 		intel_de_write(dev_priv, dss_ctl2_reg(crtc, old_crtc_state->cpu_transcoder), 0);
 	}
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index c73758d18b6f..925e0bd8bb72 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -2284,7 +2284,7 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
 
 	drm_WARN_ON(dev, pipe != crtc->pipe);
 
-	if (crtc_state->bigjoiner) {
+	if (crtc_state->bigjoiner_pipes) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Unsupported bigjoiner configuration for initial FB\n");
 		return;
-- 
2.34.1

Re: [Intel-gfx] [PATCH 10/12] drm/i915: Eliminate bigjoiner boolean

[Nautiyal, Ankit K]

On 2/16/2022 12:02 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Since we now have the bigjoiner_pipes bitmask the boolean
> is redundant. Get rid of it.
>
> Also, populating bigjoiner_pipes already during
> encoder->compute_config() allows us to use it much earlier
> during the state calculation as well. The initial aim is
> to use it in intel_crtc_compute_config().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
>   drivers/gpu/drm/i915/display/intel_display.c  | 50 ++++++++-----------
>   .../drm/i915/display/intel_display_debugfs.c  |  2 +-
>   .../drm/i915/display/intel_display_types.h    |  3 --
>   drivers/gpu/drm/i915/display/intel_dp.c       | 13 ++---
>   drivers/gpu/drm/i915/display/intel_vdsc.c     |  8 +--
>   .../drm/i915/display/skl_universal_plane.c    |  2 +-
>   7 files changed, 36 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 1f448f4e9aaf..da6cf0515164 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -640,7 +640,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
>   	 * FIXME bigjoiner fastpath would be good
>   	 */
>   	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> -	    crtc_state->update_pipe || crtc_state->bigjoiner)
> +	    crtc_state->update_pipe || crtc_state->bigjoiner_pipes)
>   		goto slow;
>   
>   	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 47b5d8cc16fd..192474163edb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1926,7 +1926,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>   	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
>   		return;
>   
> -	if (!new_crtc_state->bigjoiner) {
> +	if (!new_crtc_state->bigjoiner_pipes) {
>   		intel_encoders_pre_pll_enable(state, crtc);
>   
>   		if (new_crtc_state->shared_dpll)
> @@ -2727,7 +2727,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>   static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>   					   struct drm_display_mode *mode)
>   {
> -	if (!crtc_state->bigjoiner)
> +	if (!crtc_state->bigjoiner_pipes)
>   		return;
>   
>   	mode->crtc_clock /= 2;
> @@ -2811,7 +2811,7 @@ static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state
>   {
>   	int width, height;
>   
> -	if (!crtc_state->bigjoiner)
> +	if (!crtc_state->bigjoiner_pipes)
>   		return;
>   
>   	width = drm_rect_width(&crtc_state->pipe_src);
> @@ -4218,7 +4218,6 @@ static void intel_bigjoiner_get_config(struct intel_crtc_state *crtc_state)
>   	if (((master_pipes | slave_pipes) & BIT(pipe)) == 0)
>   		return;
>   
> -	crtc_state->bigjoiner = true;
>   	crtc_state->bigjoiner_pipes =
>   		BIT(get_bigjoiner_master_pipe(pipe, master_pipes, slave_pipes)) |
>   		get_bigjoiner_slave_pipes(pipe, master_pipes, slave_pipes);

Although not part of this patch, do we need to check if 
get_bigjoiner_master_pipe() does not give PIPE_INVALID?

Perhaps in a case where master_pipe is read as 0 but some garbage for 
slave_pipes during readout?

Should there be a check for INVALID_PIPE, before feeding into BIT() macro?

Otherwise patch looks good to me.

Regards,

Ankit

> @@ -5804,6 +5803,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
>   		intel_atomic_get_new_crtc_state(state, master_crtc);
>   	struct intel_crtc_state *saved_state;
>   
> +	WARN_ON(master_crtc_state->bigjoiner_pipes !=
> +		slave_crtc_state->bigjoiner_pipes);
> +
>   	saved_state = kmemdup(master_crtc_state, sizeof(*saved_state), GFP_KERNEL);
>   	if (!saved_state)
>   		return -ENOMEM;
> @@ -5834,6 +5836,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
>   	slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
>   	slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
>   
> +	WARN_ON(master_crtc_state->bigjoiner_pipes !=
> +		slave_crtc_state->bigjoiner_pipes);
> +
>   	return 0;
>   }
>   
> @@ -6604,7 +6609,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>   
>   	PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
>   	PIPE_CONF_CHECK_I(master_transcoder);
> -	PIPE_CONF_CHECK_BOOL(bigjoiner);
>   	PIPE_CONF_CHECK_X(bigjoiner_pipes);
>   
>   	PIPE_CONF_CHECK_I(dsc.compression_enable);
> @@ -7535,32 +7539,26 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
>   	struct intel_crtc_state *master_crtc_state =
>   		intel_atomic_get_new_crtc_state(state, master_crtc);
>   	struct intel_crtc *slave_crtc;
> -	u8 slave_pipes;
>   
> -	/*
> -	 * TODO: encoder.compute_config() may be the best
> -	 * place to populate the bitmask for the master crtc.
> -	 * For now encoder.compute_config() just flags things
> -	 * as needing bigjoiner and we populate the bitmask
> -	 * here.
> -	 */
> -	WARN_ON(master_crtc_state->bigjoiner_pipes);
> -
> -	if (!master_crtc_state->bigjoiner)
> +	if (!master_crtc_state->bigjoiner_pipes)
>   		return 0;
>   
> -	slave_pipes = BIT(master_crtc->pipe + 1);
> +	/* sanity check */
> +	if (drm_WARN_ON(&i915->drm,
> +			master_crtc->pipe != bigjoiner_master_pipe(master_crtc_state)))
> +		return -EINVAL;
>   
> -	if (slave_pipes & ~bigjoiner_pipes(i915)) {
> +	if (master_crtc_state->bigjoiner_pipes & ~bigjoiner_pipes(i915)) {
>   		drm_dbg_kms(&i915->drm,
>   			    "[CRTC:%d:%s] Cannot act as big joiner master "
> -			    "(need 0x%x as slave pipes, only 0x%x possible)\n",
> +			    "(need 0x%x as pipes, only 0x%x possible)\n",
>   			    master_crtc->base.base.id, master_crtc->base.name,
> -			    slave_pipes, bigjoiner_pipes(i915));
> +			    master_crtc_state->bigjoiner_pipes, bigjoiner_pipes(i915));
>   		return -EINVAL;
>   	}
>   
> -	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc, slave_pipes) {
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
>   		struct intel_crtc_state *slave_crtc_state;
>   		int ret;
>   
> @@ -7594,10 +7592,8 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
>   			    slave_crtc->base.base.id, slave_crtc->base.name,
>   			    master_crtc->base.base.id, master_crtc->base.name);
>   
> -		master_crtc_state->bigjoiner_pipes =
> -			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
>   		slave_crtc_state->bigjoiner_pipes =
> -			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
> +			master_crtc_state->bigjoiner_pipes;
>   
>   		ret = copy_bigjoiner_crtc_state_modeset(state, slave_crtc);
>   		if (ret)
> @@ -7620,13 +7616,11 @@ static void kill_bigjoiner_slave(struct intel_atomic_state *state,
>   		struct intel_crtc_state *slave_crtc_state =
>   			intel_atomic_get_new_crtc_state(state, slave_crtc);
>   
> -		slave_crtc_state->bigjoiner = false;
>   		slave_crtc_state->bigjoiner_pipes = 0;
>   
>   		intel_crtc_copy_uapi_to_hw_state_modeset(state, slave_crtc);
>   	}
>   
> -	master_crtc_state->bigjoiner = false;
>   	master_crtc_state->bigjoiner_pipes = 0;
>   }
>   
> @@ -7936,7 +7930,7 @@ static int intel_atomic_check(struct drm_device *dev,
>   			}
>   		}
>   
> -		if (new_crtc_state->bigjoiner) {
> +		if (new_crtc_state->bigjoiner_pipes) {
>   			if (intel_pipes_need_modeset(state, new_crtc_state->bigjoiner_pipes)) {
>   				new_crtc_state->uapi.mode_changed = true;
>   				new_crtc_state->update_pipe = false;
> @@ -10338,7 +10332,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>   			intel_encoder_get_config(encoder, crtc_state);
>   
>   			/* read out to slave crtc as well for bigjoiner */
> -			if (crtc_state->bigjoiner) {
> +			if (crtc_state->bigjoiner_pipes) {
>   				struct intel_crtc *slave_crtc;
>   
>   				/* encoder should read be linked to bigjoiner master */
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 475ae7e7f760..c87718ae2c46 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -935,7 +935,7 @@ static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
>   		intel_scaler_info(m, crtc);
>   	}
>   
> -	if (crtc_state->bigjoiner)
> +	if (crtc_state->bigjoiner_pipes)
>   		seq_printf(m, "\tLinked to 0x%x pipes as a %s\n",
>   			   crtc_state->bigjoiner_pipes,
>   			   intel_crtc_is_bigjoiner_slave(crtc_state) ? "slave" : "master");
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index c1a291b6b638..92357fdbd0f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1199,9 +1199,6 @@ struct intel_crtc_state {
>   	/* enable pipe csc? */
>   	bool csc_enable;
>   
> -	/* enable pipe big joiner? */
> -	bool bigjoiner;
> -
>   	/* big joiner pipe bitmask */
>   	u8 bigjoiner_pipes;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1046e7fe310a..05e1da3c43e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1424,13 +1424,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>   						    pipe_config->lane_count,
>   						    adjusted_mode->crtc_clock,
>   						    adjusted_mode->crtc_hdisplay,
> -						    pipe_config->bigjoiner,
> +						    pipe_config->bigjoiner_pipes,
>   						    pipe_bpp);
>   		dsc_dp_slice_count =
>   			intel_dp_dsc_get_slice_count(intel_dp,
>   						     adjusted_mode->crtc_clock,
>   						     adjusted_mode->crtc_hdisplay,
> -						     pipe_config->bigjoiner);
> +						     pipe_config->bigjoiner_pipes);
>   		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
>   			drm_dbg_kms(&dev_priv->drm,
>   				    "Compressed BPP/Slice Count not supported\n");
> @@ -1464,7 +1464,7 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>   	 * then we need to use 2 VDSC instances.
>   	 */
>   	if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq ||
> -	    pipe_config->bigjoiner) {
> +	    pipe_config->bigjoiner_pipes) {
>   		if (pipe_config->dsc.slice_count < 2) {
>   			drm_dbg_kms(&dev_priv->drm,
>   				    "Cannot split stream to use 2 VDSC instances\n");
> @@ -1500,6 +1500,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>   			     struct drm_connector_state *conn_state)
>   {
>   	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>   	const struct drm_display_mode *adjusted_mode =
>   		&pipe_config->hw.adjusted_mode;
>   	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -1537,7 +1538,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>   
>   	if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay,
>   				    adjusted_mode->crtc_clock))
> -		pipe_config->bigjoiner = true;
> +		pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>   
>   	/*
>   	 * Optimize for slow and wide for everything, because there are some
> @@ -1550,8 +1551,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>   	 * onwards pipe joiner can be enabled without compression.
>   	 */
>   	drm_dbg_kms(&i915->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
> -	if (ret || intel_dp->force_dsc_en || (DISPLAY_VER(i915) < 13 &&
> -					      pipe_config->bigjoiner)) {
> +	if (ret || intel_dp->force_dsc_en ||
> +	    (DISPLAY_VER(i915) < 13 && pipe_config->bigjoiner_pipes)) {
>   		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
>   						  conn_state, &limits);
>   		if (ret < 0)
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 545eff5bf158..28a1c982750e 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -579,7 +579,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
>   	u8 num_vdsc_instances = (crtc_state->dsc.dsc_split) ? 2 : 1;
>   	int i = 0;
>   
> -	if (crtc_state->bigjoiner)
> +	if (crtc_state->bigjoiner_pipes)
>   		num_vdsc_instances *= 2;
>   
>   	/* Populate PICTURE_PARAMETER_SET_0 registers */
> @@ -1113,7 +1113,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
>   	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>   	u32 dss_ctl1_val = 0;
>   
> -	if (crtc_state->bigjoiner && !crtc_state->dsc.compression_enable) {
> +	if (crtc_state->bigjoiner_pipes && !crtc_state->dsc.compression_enable) {
>   		if (intel_crtc_is_bigjoiner_slave(crtc_state))
>   			dss_ctl1_val |= UNCOMPRESSED_JOINER_SLAVE;
>   		else
> @@ -1140,7 +1140,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
>   		dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
>   		dss_ctl1_val |= JOINER_ENABLE;
>   	}
> -	if (crtc_state->bigjoiner) {
> +	if (crtc_state->bigjoiner_pipes) {
>   		dss_ctl1_val |= BIG_JOINER_ENABLE;
>   		if (!intel_crtc_is_bigjoiner_slave(crtc_state))
>   			dss_ctl1_val |= MASTER_BIG_JOINER_ENABLE;
> @@ -1156,7 +1156,7 @@ void intel_dsc_disable(const struct intel_crtc_state *old_crtc_state)
>   
>   	/* Disable only if either of them is enabled */
>   	if (old_crtc_state->dsc.compression_enable ||
> -	    old_crtc_state->bigjoiner) {
> +	    old_crtc_state->bigjoiner_pipes) {
>   		intel_de_write(dev_priv, dss_ctl1_reg(crtc, old_crtc_state->cpu_transcoder), 0);
>   		intel_de_write(dev_priv, dss_ctl2_reg(crtc, old_crtc_state->cpu_transcoder), 0);
>   	}
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index c73758d18b6f..925e0bd8bb72 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -2284,7 +2284,7 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
>   
>   	drm_WARN_ON(dev, pipe != crtc->pipe);
>   
> -	if (crtc_state->bigjoiner) {
> +	if (crtc_state->bigjoiner_pipes) {
>   		drm_dbg_kms(&dev_priv->drm,
>   			    "Unsupported bigjoiner configuration for initial FB\n");
>   		return;

Re: [Intel-gfx] [PATCH 10/12] drm/i915: Eliminate bigjoiner boolean

[Ville Syrjälä]

On Wed, Feb 16, 2022 at 04:27:49PM +0530, Nautiyal, Ankit K wrote:
> 
> On 2/16/2022 12:02 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Since we now have the bigjoiner_pipes bitmask the boolean
> > is redundant. Get rid of it.
> >
> > Also, populating bigjoiner_pipes already during
> > encoder->compute_config() allows us to use it much earlier
> > during the state calculation as well. The initial aim is
> > to use it in intel_crtc_compute_config().
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
> >   drivers/gpu/drm/i915/display/intel_display.c  | 50 ++++++++-----------
> >   .../drm/i915/display/intel_display_debugfs.c  |  2 +-
> >   .../drm/i915/display/intel_display_types.h    |  3 --
> >   drivers/gpu/drm/i915/display/intel_dp.c       | 13 ++---
> >   drivers/gpu/drm/i915/display/intel_vdsc.c     |  8 +--
> >   .../drm/i915/display/skl_universal_plane.c    |  2 +-
> >   7 files changed, 36 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> > index 1f448f4e9aaf..da6cf0515164 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> > @@ -640,7 +640,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> >   	 * FIXME bigjoiner fastpath would be good
> >   	 */
> >   	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> > -	    crtc_state->update_pipe || crtc_state->bigjoiner)
> > +	    crtc_state->update_pipe || crtc_state->bigjoiner_pipes)
> >   		goto slow;
> >   
> >   	/*
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 47b5d8cc16fd..192474163edb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1926,7 +1926,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
> >   	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> >   		return;
> >   
> > -	if (!new_crtc_state->bigjoiner) {
> > +	if (!new_crtc_state->bigjoiner_pipes) {
> >   		intel_encoders_pre_pll_enable(state, crtc);
> >   
> >   		if (new_crtc_state->shared_dpll)
> > @@ -2727,7 +2727,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> >   static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
> >   					   struct drm_display_mode *mode)
> >   {
> > -	if (!crtc_state->bigjoiner)
> > +	if (!crtc_state->bigjoiner_pipes)
> >   		return;
> >   
> >   	mode->crtc_clock /= 2;
> > @@ -2811,7 +2811,7 @@ static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state
> >   {
> >   	int width, height;
> >   
> > -	if (!crtc_state->bigjoiner)
> > +	if (!crtc_state->bigjoiner_pipes)
> >   		return;
> >   
> >   	width = drm_rect_width(&crtc_state->pipe_src);
> > @@ -4218,7 +4218,6 @@ static void intel_bigjoiner_get_config(struct intel_crtc_state *crtc_state)
> >   	if (((master_pipes | slave_pipes) & BIT(pipe)) == 0)
> >   		return;
> >   
> > -	crtc_state->bigjoiner = true;
> >   	crtc_state->bigjoiner_pipes =
> >   		BIT(get_bigjoiner_master_pipe(pipe, master_pipes, slave_pipes)) |
> >   		get_bigjoiner_slave_pipes(pipe, master_pipes, slave_pipes);
> 
> Although not part of this patch, do we need to check if 
> get_bigjoiner_master_pipe() does not give PIPE_INVALID?
> 
> Perhaps in a case where master_pipe is read as 0 but some garbage for 
> slave_pipes during readout?
> 
> Should there be a check for INVALID_PIPE, before feeding into BIT() macro?

I think if we want to do more thourough validation against totally bogus
hardware programming then we should just do it once at the start.
enabled_bigjoiner_pipes() does have something, but it's only good for
the two joined pipes cases. Also it just warns and doesn't do anything
more than that atm. The simple option might be to make it just zero out
the masks entirely if they look totally bogus. The readout would then
be skipped for all slave pipes.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 10/12] drm/i915: Eliminate bigjoiner boolean

[Nautiyal, Ankit K]

On 2/16/2022 4:34 PM, Ville Syrjälä wrote:
> On Wed, Feb 16, 2022 at 04:27:49PM +0530, Nautiyal, Ankit K wrote:
>> On 2/16/2022 12:02 AM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Since we now have the bigjoiner_pipes bitmask the boolean
>>> is redundant. Get rid of it.
>>>
>>> Also, populating bigjoiner_pipes already during
>>> encoder->compute_config() allows us to use it much earlier
>>> during the state calculation as well. The initial aim is
>>> to use it in intel_crtc_compute_config().
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
>>>    drivers/gpu/drm/i915/display/intel_display.c  | 50 ++++++++-----------
>>>    .../drm/i915/display/intel_display_debugfs.c  |  2 +-
>>>    .../drm/i915/display/intel_display_types.h    |  3 --
>>>    drivers/gpu/drm/i915/display/intel_dp.c       | 13 ++---
>>>    drivers/gpu/drm/i915/display/intel_vdsc.c     |  8 +--
>>>    .../drm/i915/display/skl_universal_plane.c    |  2 +-
>>>    7 files changed, 36 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
>>> index 1f448f4e9aaf..da6cf0515164 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
>>> @@ -640,7 +640,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
>>>    	 * FIXME bigjoiner fastpath would be good
>>>    	 */
>>>    	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
>>> -	    crtc_state->update_pipe || crtc_state->bigjoiner)
>>> +	    crtc_state->update_pipe || crtc_state->bigjoiner_pipes)
>>>    		goto slow;
>>>    
>>>    	/*
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 47b5d8cc16fd..192474163edb 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -1926,7 +1926,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>>>    	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
>>>    		return;
>>>    
>>> -	if (!new_crtc_state->bigjoiner) {
>>> +	if (!new_crtc_state->bigjoiner_pipes) {
>>>    		intel_encoders_pre_pll_enable(state, crtc);
>>>    
>>>    		if (new_crtc_state->shared_dpll)
>>> @@ -2727,7 +2727,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>>>    static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>>>    					   struct drm_display_mode *mode)
>>>    {
>>> -	if (!crtc_state->bigjoiner)
>>> +	if (!crtc_state->bigjoiner_pipes)
>>>    		return;
>>>    
>>>    	mode->crtc_clock /= 2;
>>> @@ -2811,7 +2811,7 @@ static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state
>>>    {
>>>    	int width, height;
>>>    
>>> -	if (!crtc_state->bigjoiner)
>>> +	if (!crtc_state->bigjoiner_pipes)
>>>    		return;
>>>    
>>>    	width = drm_rect_width(&crtc_state->pipe_src);
>>> @@ -4218,7 +4218,6 @@ static void intel_bigjoiner_get_config(struct intel_crtc_state *crtc_state)
>>>    	if (((master_pipes | slave_pipes) & BIT(pipe)) == 0)
>>>    		return;
>>>    
>>> -	crtc_state->bigjoiner = true;
>>>    	crtc_state->bigjoiner_pipes =
>>>    		BIT(get_bigjoiner_master_pipe(pipe, master_pipes, slave_pipes)) |
>>>    		get_bigjoiner_slave_pipes(pipe, master_pipes, slave_pipes);
>> Although not part of this patch, do we need to check if
>> get_bigjoiner_master_pipe() does not give PIPE_INVALID?
>>
>> Perhaps in a case where master_pipe is read as 0 but some garbage for
>> slave_pipes during readout?
>>
>> Should there be a check for INVALID_PIPE, before feeding into BIT() macro?
> I think if we want to do more thourough validation against totally bogus
> hardware programming then we should just do it once at the start.
> enabled_bigjoiner_pipes() does have something, but it's only good for
> the two joined pipes cases. Also it just warns and doesn't do anything
> more than that atm. The simple option might be to make it just zero out
> the masks entirely if they look totally bogus. The readout would then
> be skipped for all slave pipes.

Yes you are right, enabled_bigjoiner_pipes does have a check in the end 
and that will prevent bogus value to

a certain extent. Given case would not occur, atleast for two joined 
pipes case.

Anyways, the patch seems to be straight forward and looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Re: [Intel-gfx] [PATCH 10/12] drm/i915: Eliminate bigjoiner boolean

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:06PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since we now have the bigjoiner_pipes bitmask the boolean
> is redundant. Get rid of it.
> 
> Also, populating bigjoiner_pipes already during
> encoder->compute_config() allows us to use it much earlier
> during the state calculation as well. The initial aim is
> to use it in intel_crtc_compute_config().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c   |  2 +-
>  drivers/gpu/drm/i915/display/intel_display.c  | 50 ++++++++-----------
>  .../drm/i915/display/intel_display_debugfs.c  |  2 +-
>  .../drm/i915/display/intel_display_types.h    |  3 --
>  drivers/gpu/drm/i915/display/intel_dp.c       | 13 ++---
>  drivers/gpu/drm/i915/display/intel_vdsc.c     |  8 +--
>  .../drm/i915/display/skl_universal_plane.c    |  2 +-
>  7 files changed, 36 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 1f448f4e9aaf..da6cf0515164 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -640,7 +640,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
>  	 * FIXME bigjoiner fastpath would be good
>  	 */
>  	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> -	    crtc_state->update_pipe || crtc_state->bigjoiner)
> +	    crtc_state->update_pipe || crtc_state->bigjoiner_pipes)
>  		goto slow;
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 47b5d8cc16fd..192474163edb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1926,7 +1926,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>  	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
>  		return;
>  
> -	if (!new_crtc_state->bigjoiner) {
> +	if (!new_crtc_state->bigjoiner_pipes) {
>  		intel_encoders_pre_pll_enable(state, crtc);
>  
>  		if (new_crtc_state->shared_dpll)
> @@ -2727,7 +2727,7 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>  					   struct drm_display_mode *mode)
>  {
> -	if (!crtc_state->bigjoiner)
> +	if (!crtc_state->bigjoiner_pipes)
>  		return;
>  
>  	mode->crtc_clock /= 2;
> @@ -2811,7 +2811,7 @@ static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state
>  {
>  	int width, height;
>  
> -	if (!crtc_state->bigjoiner)
> +	if (!crtc_state->bigjoiner_pipes)
>  		return;
>  
>  	width = drm_rect_width(&crtc_state->pipe_src);
> @@ -4218,7 +4218,6 @@ static void intel_bigjoiner_get_config(struct intel_crtc_state *crtc_state)
>  	if (((master_pipes | slave_pipes) & BIT(pipe)) == 0)
>  		return;
>  
> -	crtc_state->bigjoiner = true;
>  	crtc_state->bigjoiner_pipes =
>  		BIT(get_bigjoiner_master_pipe(pipe, master_pipes, slave_pipes)) |
>  		get_bigjoiner_slave_pipes(pipe, master_pipes, slave_pipes);
> @@ -5804,6 +5803,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, master_crtc);
>  	struct intel_crtc_state *saved_state;
>  
> +	WARN_ON(master_crtc_state->bigjoiner_pipes !=
> +		slave_crtc_state->bigjoiner_pipes);
> +
>  	saved_state = kmemdup(master_crtc_state, sizeof(*saved_state), GFP_KERNEL);
>  	if (!saved_state)
>  		return -ENOMEM;
> @@ -5834,6 +5836,9 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state,
>  	slave_crtc_state->uapi.connectors_changed = master_crtc_state->uapi.connectors_changed;
>  	slave_crtc_state->uapi.active_changed = master_crtc_state->uapi.active_changed;
>  
> +	WARN_ON(master_crtc_state->bigjoiner_pipes !=
> +		slave_crtc_state->bigjoiner_pipes);
> +
>  	return 0;
>  }
>  
> @@ -6604,7 +6609,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  
>  	PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
>  	PIPE_CONF_CHECK_I(master_transcoder);
> -	PIPE_CONF_CHECK_BOOL(bigjoiner);
>  	PIPE_CONF_CHECK_X(bigjoiner_pipes);
>  
>  	PIPE_CONF_CHECK_I(dsc.compression_enable);
> @@ -7535,32 +7539,26 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
>  	struct intel_crtc_state *master_crtc_state =
>  		intel_atomic_get_new_crtc_state(state, master_crtc);
>  	struct intel_crtc *slave_crtc;
> -	u8 slave_pipes;
>  
> -	/*
> -	 * TODO: encoder.compute_config() may be the best
> -	 * place to populate the bitmask for the master crtc.
> -	 * For now encoder.compute_config() just flags things
> -	 * as needing bigjoiner and we populate the bitmask
> -	 * here.
> -	 */
> -	WARN_ON(master_crtc_state->bigjoiner_pipes);
> -
> -	if (!master_crtc_state->bigjoiner)
> +	if (!master_crtc_state->bigjoiner_pipes)
>  		return 0;
>  
> -	slave_pipes = BIT(master_crtc->pipe + 1);
> +	/* sanity check */
> +	if (drm_WARN_ON(&i915->drm,
> +			master_crtc->pipe != bigjoiner_master_pipe(master_crtc_state)))
> +		return -EINVAL;
>  
> -	if (slave_pipes & ~bigjoiner_pipes(i915)) {
> +	if (master_crtc_state->bigjoiner_pipes & ~bigjoiner_pipes(i915)) {

So here we are making sure that in compute_config if master pipe = D and we blidly set
genmask to have slave pipe = D + 1 = E then here this will catch it and fail the atomic check
Is that undestanding correct?

With this and the concern that Ankit brought, but then we have that check in enabled_bigjoiner_pipes()

Eeverything else makes sense

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

>  		drm_dbg_kms(&i915->drm,
>  			    "[CRTC:%d:%s] Cannot act as big joiner master "
> -			    "(need 0x%x as slave pipes, only 0x%x possible)\n",
> +			    "(need 0x%x as pipes, only 0x%x possible)\n",
>  			    master_crtc->base.base.id, master_crtc->base.name,
> -			    slave_pipes, bigjoiner_pipes(i915));
> +			    master_crtc_state->bigjoiner_pipes, bigjoiner_pipes(i915));
>  		return -EINVAL;
>  	}
>  
> -	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc, slave_pipes) {
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state)) {
>  		struct intel_crtc_state *slave_crtc_state;
>  		int ret;
>  
> @@ -7594,10 +7592,8 @@ static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state,
>  			    slave_crtc->base.base.id, slave_crtc->base.name,
>  			    master_crtc->base.base.id, master_crtc->base.name);
>  
> -		master_crtc_state->bigjoiner_pipes =
> -			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
>  		slave_crtc_state->bigjoiner_pipes =
> -			BIT(master_crtc->pipe) | BIT(slave_crtc->pipe);
> +			master_crtc_state->bigjoiner_pipes;
>  
>  		ret = copy_bigjoiner_crtc_state_modeset(state, slave_crtc);
>  		if (ret)
> @@ -7620,13 +7616,11 @@ static void kill_bigjoiner_slave(struct intel_atomic_state *state,
>  		struct intel_crtc_state *slave_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, slave_crtc);
>  
> -		slave_crtc_state->bigjoiner = false;
>  		slave_crtc_state->bigjoiner_pipes = 0;
>  
>  		intel_crtc_copy_uapi_to_hw_state_modeset(state, slave_crtc);
>  	}
>  
> -	master_crtc_state->bigjoiner = false;
>  	master_crtc_state->bigjoiner_pipes = 0;
>  }
>  
> @@ -7936,7 +7930,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  			}
>  		}
>  
> -		if (new_crtc_state->bigjoiner) {
> +		if (new_crtc_state->bigjoiner_pipes) {
>  			if (intel_pipes_need_modeset(state, new_crtc_state->bigjoiner_pipes)) {
>  				new_crtc_state->uapi.mode_changed = true;
>  				new_crtc_state->update_pipe = false;
> @@ -10338,7 +10332,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			intel_encoder_get_config(encoder, crtc_state);
>  
>  			/* read out to slave crtc as well for bigjoiner */
> -			if (crtc_state->bigjoiner) {
> +			if (crtc_state->bigjoiner_pipes) {
>  				struct intel_crtc *slave_crtc;
>  
>  				/* encoder should read be linked to bigjoiner master */
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 475ae7e7f760..c87718ae2c46 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -935,7 +935,7 @@ static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
>  		intel_scaler_info(m, crtc);
>  	}
>  
> -	if (crtc_state->bigjoiner)
> +	if (crtc_state->bigjoiner_pipes)
>  		seq_printf(m, "\tLinked to 0x%x pipes as a %s\n",
>  			   crtc_state->bigjoiner_pipes,
>  			   intel_crtc_is_bigjoiner_slave(crtc_state) ? "slave" : "master");
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index c1a291b6b638..92357fdbd0f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1199,9 +1199,6 @@ struct intel_crtc_state {
>  	/* enable pipe csc? */
>  	bool csc_enable;
>  
> -	/* enable pipe big joiner? */
> -	bool bigjoiner;
> -
>  	/* big joiner pipe bitmask */
>  	u8 bigjoiner_pipes;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1046e7fe310a..05e1da3c43e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1424,13 +1424,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  						    pipe_config->lane_count,
>  						    adjusted_mode->crtc_clock,
>  						    adjusted_mode->crtc_hdisplay,
> -						    pipe_config->bigjoiner,
> +						    pipe_config->bigjoiner_pipes,
>  						    pipe_bpp);
>  		dsc_dp_slice_count =
>  			intel_dp_dsc_get_slice_count(intel_dp,
>  						     adjusted_mode->crtc_clock,
>  						     adjusted_mode->crtc_hdisplay,
> -						     pipe_config->bigjoiner);
> +						     pipe_config->bigjoiner_pipes);
>  		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
>  			drm_dbg_kms(&dev_priv->drm,
>  				    "Compressed BPP/Slice Count not supported\n");
> @@ -1464,7 +1464,7 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	 * then we need to use 2 VDSC instances.
>  	 */
>  	if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq ||
> -	    pipe_config->bigjoiner) {
> +	    pipe_config->bigjoiner_pipes) {
>  		if (pipe_config->dsc.slice_count < 2) {
>  			drm_dbg_kms(&dev_priv->drm,
>  				    "Cannot split stream to use 2 VDSC instances\n");
> @@ -1500,6 +1500,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  			     struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>  	const struct drm_display_mode *adjusted_mode =
>  		&pipe_config->hw.adjusted_mode;
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -1537,7 +1538,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  
>  	if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay,
>  				    adjusted_mode->crtc_clock))
> -		pipe_config->bigjoiner = true;
> +		pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>  
>  	/*
>  	 * Optimize for slow and wide for everything, because there are some
> @@ -1550,8 +1551,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	 * onwards pipe joiner can be enabled without compression.
>  	 */
>  	drm_dbg_kms(&i915->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
> -	if (ret || intel_dp->force_dsc_en || (DISPLAY_VER(i915) < 13 &&
> -					      pipe_config->bigjoiner)) {
> +	if (ret || intel_dp->force_dsc_en ||
> +	    (DISPLAY_VER(i915) < 13 && pipe_config->bigjoiner_pipes)) {
>  		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
>  						  conn_state, &limits);
>  		if (ret < 0)
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 545eff5bf158..28a1c982750e 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -579,7 +579,7 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
>  	u8 num_vdsc_instances = (crtc_state->dsc.dsc_split) ? 2 : 1;
>  	int i = 0;
>  
> -	if (crtc_state->bigjoiner)
> +	if (crtc_state->bigjoiner_pipes)
>  		num_vdsc_instances *= 2;
>  
>  	/* Populate PICTURE_PARAMETER_SET_0 registers */
> @@ -1113,7 +1113,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	u32 dss_ctl1_val = 0;
>  
> -	if (crtc_state->bigjoiner && !crtc_state->dsc.compression_enable) {
> +	if (crtc_state->bigjoiner_pipes && !crtc_state->dsc.compression_enable) {
>  		if (intel_crtc_is_bigjoiner_slave(crtc_state))
>  			dss_ctl1_val |= UNCOMPRESSED_JOINER_SLAVE;
>  		else
> @@ -1140,7 +1140,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
>  		dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
>  		dss_ctl1_val |= JOINER_ENABLE;
>  	}
> -	if (crtc_state->bigjoiner) {
> +	if (crtc_state->bigjoiner_pipes) {
>  		dss_ctl1_val |= BIG_JOINER_ENABLE;
>  		if (!intel_crtc_is_bigjoiner_slave(crtc_state))
>  			dss_ctl1_val |= MASTER_BIG_JOINER_ENABLE;
> @@ -1156,7 +1156,7 @@ void intel_dsc_disable(const struct intel_crtc_state *old_crtc_state)
>  
>  	/* Disable only if either of them is enabled */
>  	if (old_crtc_state->dsc.compression_enable ||
> -	    old_crtc_state->bigjoiner) {
> +	    old_crtc_state->bigjoiner_pipes) {
>  		intel_de_write(dev_priv, dss_ctl1_reg(crtc, old_crtc_state->cpu_transcoder), 0);
>  		intel_de_write(dev_priv, dss_ctl2_reg(crtc, old_crtc_state->cpu_transcoder), 0);
>  	}
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index c73758d18b6f..925e0bd8bb72 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -2284,7 +2284,7 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
>  
>  	drm_WARN_ON(dev, pipe != crtc->pipe);
>  
> -	if (crtc_state->bigjoiner) {
> +	if (crtc_state->bigjoiner_pipes) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "Unsupported bigjoiner configuration for initial FB\n");
>  		return;
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 11/12] drm/i915: Use bigjoiner_pipes more

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace the hardcoded 2 pipe assumptions when we're massaging
pipe_mode and the pipe_src rect to be suitable for bigjoiner.
Instead we can just count the number of pipes in the bitmask.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 23 +++++++++++---------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 192474163edb..0690470eab97 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2727,16 +2727,18 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
 static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
 					   struct drm_display_mode *mode)
 {
-	if (!crtc_state->bigjoiner_pipes)
+	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
+
+	if (num_pipes < 2)
 		return;
 
-	mode->crtc_clock /= 2;
-	mode->crtc_hdisplay /= 2;
-	mode->crtc_hblank_start /= 2;
-	mode->crtc_hblank_end /= 2;
-	mode->crtc_hsync_start /= 2;
-	mode->crtc_hsync_end /= 2;
-	mode->crtc_htotal /= 2;
+	mode->crtc_clock /= num_pipes;
+	mode->crtc_hdisplay /= num_pipes;
+	mode->crtc_hblank_start /= num_pipes;
+	mode->crtc_hblank_end /= num_pipes;
+	mode->crtc_hsync_start /= num_pipes;
+	mode->crtc_hsync_end /= num_pipes;
+	mode->crtc_htotal /= num_pipes;
 }
 
 static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
@@ -2809,16 +2811,17 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
 
 static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
 {
+	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
 	int width, height;
 
-	if (!crtc_state->bigjoiner_pipes)
+	if (num_pipes < 2)
 		return;
 
 	width = drm_rect_width(&crtc_state->pipe_src);
 	height = drm_rect_height(&crtc_state->pipe_src);
 
 	drm_rect_init(&crtc_state->pipe_src, 0, 0,
-		      width / 2, height);
+		      width / num_pipes, height);
 }
 
 static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
-- 
2.34.1

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Use bigjoiner_pipes more

[Nautiyal, Ankit K]

LGTM.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

On 2/16/2022 12:02 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace the hardcoded 2 pipe assumptions when we're massaging
> pipe_mode and the pipe_src rect to be suitable for bigjoiner.
> Instead we can just count the number of pipes in the bitmask.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display.c | 23 +++++++++++---------
>   1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 192474163edb..0690470eab97 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2727,16 +2727,18 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>   static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>   					   struct drm_display_mode *mode)
>   {
> -	if (!crtc_state->bigjoiner_pipes)
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
> +
> +	if (num_pipes < 2)
>   		return;
>   
> -	mode->crtc_clock /= 2;
> -	mode->crtc_hdisplay /= 2;
> -	mode->crtc_hblank_start /= 2;
> -	mode->crtc_hblank_end /= 2;
> -	mode->crtc_hsync_start /= 2;
> -	mode->crtc_hsync_end /= 2;
> -	mode->crtc_htotal /= 2;
> +	mode->crtc_clock /= num_pipes;
> +	mode->crtc_hdisplay /= num_pipes;
> +	mode->crtc_hblank_start /= num_pipes;
> +	mode->crtc_hblank_end /= num_pipes;
> +	mode->crtc_hsync_start /= num_pipes;
> +	mode->crtc_hsync_end /= num_pipes;
> +	mode->crtc_htotal /= num_pipes;
>   }
>   
>   static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
> @@ -2809,16 +2811,17 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
>   
>   static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
>   {
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
>   	int width, height;
>   
> -	if (!crtc_state->bigjoiner_pipes)
> +	if (num_pipes < 2)
>   		return;
>   
>   	width = drm_rect_width(&crtc_state->pipe_src);
>   	height = drm_rect_height(&crtc_state->pipe_src);
>   
>   	drm_rect_init(&crtc_state->pipe_src, 0, 0,
> -		      width / 2, height);
> +		      width / num_pipes, height);
>   }
>   
>   static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Use bigjoiner_pipes more

[Ville Syrjälä]

On Tue, Feb 15, 2022 at 08:32:07PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the hardcoded 2 pipe assumptions when we're massaging
> pipe_mode and the pipe_src rect to be suitable for bigjoiner.
> Instead we can just count the number of pipes in the bitmask.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 23 +++++++++++---------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 192474163edb..0690470eab97 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2727,16 +2727,18 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>  					   struct drm_display_mode *mode)
>  {
> -	if (!crtc_state->bigjoiner_pipes)
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);

Also just occurred to me that we might want to abstract that a bit
more  since it gets repeated in several places. Could be a followup
too I suppose.

> +
> +	if (num_pipes < 2)
>  		return;
>  
> -	mode->crtc_clock /= 2;
> -	mode->crtc_hdisplay /= 2;
> -	mode->crtc_hblank_start /= 2;
> -	mode->crtc_hblank_end /= 2;
> -	mode->crtc_hsync_start /= 2;
> -	mode->crtc_hsync_end /= 2;
> -	mode->crtc_htotal /= 2;
> +	mode->crtc_clock /= num_pipes;
> +	mode->crtc_hdisplay /= num_pipes;
> +	mode->crtc_hblank_start /= num_pipes;
> +	mode->crtc_hblank_end /= num_pipes;
> +	mode->crtc_hsync_start /= num_pipes;
> +	mode->crtc_hsync_end /= num_pipes;
> +	mode->crtc_htotal /= num_pipes;
>  }
>  
>  static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
> @@ -2809,16 +2811,17 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
>  
>  static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
>  {
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
>  	int width, height;
>  
> -	if (!crtc_state->bigjoiner_pipes)
> +	if (num_pipes < 2)
>  		return;
>  
>  	width = drm_rect_width(&crtc_state->pipe_src);
>  	height = drm_rect_height(&crtc_state->pipe_src);
>  
>  	drm_rect_init(&crtc_state->pipe_src, 0, 0,
> -		      width / 2, height);
> +		      width / num_pipes, height);
>  }
>  
>  static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 11/12] drm/i915: Use bigjoiner_pipes more

[Navare, Manasi]

On Tue, Feb 15, 2022 at 08:32:07PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the hardcoded 2 pipe assumptions when we're massaging
> pipe_mode and the pipe_src rect to be suitable for bigjoiner.
> Instead we can just count the number of pipes in the bitmask.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Patch looks good, perhaps can be squashed with Patch 10 ?
But either ways

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 23 +++++++++++---------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 192474163edb..0690470eab97 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2727,16 +2727,18 @@ static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
>  static void intel_bigjoiner_adjust_timings(const struct intel_crtc_state *crtc_state,
>  					   struct drm_display_mode *mode)
>  {
> -	if (!crtc_state->bigjoiner_pipes)
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
> +
> +	if (num_pipes < 2)
>  		return;
>  
> -	mode->crtc_clock /= 2;
> -	mode->crtc_hdisplay /= 2;
> -	mode->crtc_hblank_start /= 2;
> -	mode->crtc_hblank_end /= 2;
> -	mode->crtc_hsync_start /= 2;
> -	mode->crtc_hsync_end /= 2;
> -	mode->crtc_htotal /= 2;
> +	mode->crtc_clock /= num_pipes;
> +	mode->crtc_hdisplay /= num_pipes;
> +	mode->crtc_hblank_start /= num_pipes;
> +	mode->crtc_hblank_end /= num_pipes;
> +	mode->crtc_hsync_start /= num_pipes;
> +	mode->crtc_hsync_end /= num_pipes;
> +	mode->crtc_htotal /= num_pipes;
>  }
>  
>  static void intel_splitter_adjust_timings(const struct intel_crtc_state *crtc_state,
> @@ -2809,16 +2811,17 @@ static void intel_encoder_get_config(struct intel_encoder *encoder,
>  
>  static void intel_bigjoiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
>  {
> +	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
>  	int width, height;
>  
> -	if (!crtc_state->bigjoiner_pipes)
> +	if (num_pipes < 2)
>  		return;
>  
>  	width = drm_rect_width(&crtc_state->pipe_src);
>  	height = drm_rect_height(&crtc_state->pipe_src);
>  
>  	drm_rect_init(&crtc_state->pipe_src, 0, 0,
> -		      width / 2, height);
> +		      width / num_pipes, height);
>  }
>  
>  static int intel_crtc_compute_pipe_src(struct intel_crtc_state *crtc_state)
> -- 
> 2.34.1
> 

[Intel-gfx] [PATCH 12/12] drm/i915: Make the PIPESC rect relative to the entire bigjoiner area

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When using bigjoiner it's useful to know the offset of each
individual pipe in the whole set of joined pipes. Let's include
that information in our PIPESRC rectangle. With this we can make
the plane clipping code blissfully unaware of bigjoiner usage, as
all we have to do is remove the pipe's offset from the final plane
destination coordinates.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |  7 +++---
 drivers/gpu/drm/i915/display/intel_cursor.c   |  8 ++++---
 drivers/gpu/drm/i915/display/intel_display.c  | 21 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_overlay.c  | 22 +++++++++----------
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 4f5a17e008a5..ab25be519193 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -628,10 +628,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 		return -ERANGE;
 	}
 
-	/* right side of the image is on the slave crtc, adjust dst to match */
-	if (intel_crtc_is_bigjoiner_slave(crtc_state))
-		drm_rect_translate(dst, -drm_rect_width(&crtc_state->pipe_src), 0);
-
 	/*
 	 * FIXME: This might need further adjustment for seamless scaling
 	 * with phase information, for the 2p2 and 2p1 scenarios.
@@ -648,6 +644,9 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
 		return -EINVAL;
 	}
 
+	/* final plane coordinates will be relative to the plane's pipe */
+	drm_rect_translate(dst, -clip->x1, -clip->y1);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index da6cf0515164..9279e2783e7e 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -152,9 +152,11 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	/* Use the unclipped src/dst rectangles, which we program to hw */
 	plane_state->uapi.src = src;
 	plane_state->uapi.dst = dst;
-	if (intel_crtc_is_bigjoiner_slave(crtc_state))
-		drm_rect_translate(&plane_state->uapi.dst,
-				   -drm_rect_width(&crtc_state->pipe_src), 0);
+
+	/* final plane coordinates will be relative to the plane's pipe */
+	drm_rect_translate(&plane_state->uapi.dst,
+			   -crtc_state->pipe_src.x1,
+			   -crtc_state->pipe_src.y1);
 
 	ret = intel_cursor_check_surface(plane_state);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 0690470eab97..628599958afd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3195,6 +3195,23 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc,
 	}
 }
 
+static void intel_bigjoiner_adjust_pipe_src(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	int num_pipes = hweight8(crtc_state->bigjoiner_pipes);
+	enum pipe master_pipe, pipe = crtc->pipe;
+	int width;
+
+	if (num_pipes < 2)
+		return;
+
+	master_pipe = bigjoiner_master_pipe(crtc_state);
+	width = drm_rect_width(&crtc_state->pipe_src);
+
+	drm_rect_translate_to(&crtc_state->pipe_src,
+			      (pipe - master_pipe) * width, 0);
+}
+
 static void intel_get_pipe_src_size(struct intel_crtc *crtc,
 				    struct intel_crtc_state *pipe_config)
 {
@@ -3207,6 +3224,8 @@ static void intel_get_pipe_src_size(struct intel_crtc *crtc,
 	drm_rect_init(&pipe_config->pipe_src, 0, 0,
 		      REG_FIELD_GET(PIPESRC_WIDTH_MASK, tmp) + 1,
 		      REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1);
+
+	intel_bigjoiner_adjust_pipe_src(pipe_config);
 }
 
 static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
@@ -6034,6 +6053,8 @@ intel_modeset_pipe_config_late(struct intel_crtc_state *crtc_state)
 	struct drm_connector *connector;
 	int i;
 
+	intel_bigjoiner_adjust_pipe_src(crtc_state);
+
 	for_each_new_connector_in_state(&state->base, connector,
 					conn_state, i) {
 		struct intel_encoder *encoder =
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 631e1f1dc5e6..ee46561b5ae8 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -958,21 +958,21 @@ static void update_pfit_vscale_ratio(struct intel_overlay *overlay)
 static int check_overlay_dst(struct intel_overlay *overlay,
 			     struct drm_intel_overlay_put_image *rec)
 {
-	const struct intel_crtc_state *pipe_config =
+	const struct intel_crtc_state *crtc_state =
 		overlay->crtc->config;
-	int pipe_src_w = drm_rect_width(&pipe_config->pipe_src);
-	int pipe_src_h = drm_rect_height(&pipe_config->pipe_src);
+	struct drm_rect req, clipped;
 
-	if (rec->dst_height == 0 || rec->dst_width == 0)
-		return -EINVAL;
+	drm_rect_init(&req, rec->dst_x, rec->dst_y,
+		      rec->dst_width, rec->dst_height);
+
+	clipped = req;
+	drm_rect_intersect(&clipped, &crtc_state->pipe_src);
 
-	if (rec->dst_x < pipe_src_w &&
-	    rec->dst_x + rec->dst_width <= pipe_src_w &&
-	    rec->dst_y < pipe_src_h &&
-	    rec->dst_y + rec->dst_height <= pipe_src_h)
-		return 0;
-	else
+	if (!drm_rect_visible(&clipped) ||
+	    !drm_rect_equals(&clipped, &req))
 		return -EINVAL;
+
+	return 0;
 }
 
 static int check_overlay_scaling(struct drm_intel_overlay_put_image *rec)
-- 
2.34.1

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Move bigjoiner refactoring

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9666 bytes --]

== Series Details ==

Series: drm/i915: Move bigjoiner refactoring
URL   : https://patchwork.freedesktop.org/series/100195/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11238 -> Patchwork_22286
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_22286 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22286, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/index.html

Participating hosts (46 -> 44)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (3): fi-bsw-cyan bat-jsl-2 shard-tglu 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22286:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-hsw-4770:        [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-hsw-4770/igt@kms_force_connector_basic@force-load-detect.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-hsw-4770/igt@kms_force_connector_basic@force-load-detect.html
    - fi-ivb-3770:        [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-ivb-3770/igt@kms_force_connector_basic@force-load-detect.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-ivb-3770/igt@kms_force_connector_basic@force-load-detect.html
    - fi-blb-e6850:       [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-blb-e6850/igt@kms_force_connector_basic@force-load-detect.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-blb-e6850/igt@kms_force_connector_basic@force-load-detect.html
    - fi-bwr-2160:        [PASS][7] -> [FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-bwr-2160/igt@kms_force_connector_basic@force-load-detect.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-bwr-2160/igt@kms_force_connector_basic@force-load-detect.html
    - fi-snb-2600:        [PASS][9] -> [FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-snb-2600/igt@kms_force_connector_basic@force-load-detect.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-snb-2600/igt@kms_force_connector_basic@force-load-detect.html
    - fi-elk-e7500:       [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-elk-e7500/igt@kms_force_connector_basic@force-load-detect.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-elk-e7500/igt@kms_force_connector_basic@force-load-detect.html
    - fi-ilk-650:         [PASS][13] -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-ilk-650/igt@kms_force_connector_basic@force-load-detect.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-ilk-650/igt@kms_force_connector_basic@force-load-detect.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][15]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-pnv-d510/igt@kms_force_connector_basic@force-load-detect.html
    - fi-snb-2520m:       [PASS][16] -> [FAIL][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-snb-2520m/igt@kms_force_connector_basic@force-load-detect.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-snb-2520m/igt@kms_force_connector_basic@force-load-detect.html

  
Known issues
------------

  Here are the changes found in Patchwork_22286 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_prime@amd-to-i915:
    - fi-ivb-3770:        NOTRUN -> [SKIP][18] ([fdo#109271]) +17 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-ivb-3770/igt@amdgpu/amd_prime@amd-to-i915.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-skl-6600u:       NOTRUN -> [INCOMPLETE][19] ([i915#4547])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_huc_copy@huc-copy:
    - fi-pnv-d510:        NOTRUN -> [SKIP][20] ([fdo#109271]) +57 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-pnv-d510/igt@gem_huc_copy@huc-copy.html

  * igt@i915_selftest@live@hangcheck:
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][21] ([i915#3921])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html
    - fi-snb-2600:        [PASS][22] -> [INCOMPLETE][23] ([i915#3921])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][24] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-bdw-5557u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [PASS][25] -> [DMESG-WARN][26] ([i915#4269])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-cfl-8109u:       [PASS][27] -> [DMESG-WARN][28] ([i915#295]) +12 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  * igt@kms_psr@cursor_plane_move:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][29] ([fdo#109271]) +13 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-bdw-5557u/igt@kms_psr@cursor_plane_move.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gem_contexts:
    - {fi-tgl-dsi}:       [DMESG-WARN][30] ([i915#2867]) -> [PASS][31] +16 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-tgl-dsi/igt@i915_selftest@live@gem_contexts.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-tgl-dsi/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@hangcheck:
    - fi-ivb-3770:        [INCOMPLETE][32] ([i915#3303]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/fi-ivb-3770/igt@i915_selftest@live@hangcheck.html

  
#### Warnings ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-6:          [DMESG-FAIL][34] ([i915#4957]) -> [DMESG-FAIL][35] ([i915#4494] / [i915#4957])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11238/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/bat-dg1-6/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957


Build changes
-------------

  * Linux: CI_DRM_11238 -> Patchwork_22286

  CI-20190529: 20190529
  CI_DRM_11238: e141e36b2871c529379f7ec7d5d6ebae3137a51b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6347: 37ea4c86f97c0e05fcb6b04cff72ec927930536e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22286: 3faa720f3c7ea6ac3639ba36f966ff27b0783058 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3faa720f3c7e drm/i915: Make the PIPESC rect relative to the entire bigjoiner area
8c1c2194639e drm/i915: Use bigjoiner_pipes more
6dde6471421f drm/i915: Eliminate bigjoiner boolean
66520ca307e5 drm/i915: Start tracking PIPESRC as a drm_rect
0de07986ecda drm/i915: Fix MSO vs. bigjoiner timings confusion
a537b3ad46e2 drm/i915: Extract intel_crtc_compute_pipe_mode()
da5a00c9434c drm/i915: Extract intel_crtc_compute_pipe_src()
044dcc1af7ce drm/i915: Extract intel_bigjoiner_adjust_timings()
62729761b089 drm/i915: Extract intel_splitter_adjust_timings()
9ffcc136c6b1 drm/i915: Rename variables in intel_crtc_compute_config()
9e28803dae9f drm/i915: Remove nop bigjoiner state copy
d3284529594a drm/i915: Fix cursor coordinates on bigjoiner slave

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/index.html

[-- Attachment #2: Type: text/html, Size: 10994 bytes --]

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Move bigjoiner refactoring

[Ville Syrjälä]

On Thu, Feb 17, 2022 at 01:06:04AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Move bigjoiner refactoring
> URL   : https://patchwork.freedesktop.org/series/100195/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_11238 -> Patchwork_22286
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_22286 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_22286, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22286/index.html
> 
> Participating hosts (46 -> 44)
> ------------------------------
> 
>   Additional (1): fi-pnv-d510 
>   Missing    (3): fi-bsw-cyan bat-jsl-2 shard-tglu 
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_22286:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@kms_force_connector_basic@force-load-detect:
>     - fi-hsw-4770:        [PASS][1] -> [FAIL][2]

Grr. This one is a real issue. Sort of regression from the original
bigjoiner patches, but only clearly exposed by this pipesrc rect
stuff. More fixes incoming...

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 00/12] drm/i915: Move bigjoiner refactoring

[Nautiyal, Ankit K]

Hi,

Was able to test 8k@60/30 with the changes, where bigjoiner will come 
into play, didn't get any issues/errors.

Thanks & Regards,

Ankit

On 2/16/2022 12:01 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> This is an attempt at more or less finish the bigjoiner
> state computation/readout refactoring.
>
> Stuff that should now be in decent shape:
> - cursor should appear in the right spot on all pipes
> - plane clipping/etc. independent of number of joined pipes
>    thanks to the PIPESRC drm_rect
> - the PIPESRC drm_rect should prove helpful for the seam
>    elimination stuff too in the future, as well as for some
>    other planned scaler fixes/cleanups
> - bigjoiner vs. MSO timings should be properly handled now
>
> What is likely still busted:
> - panel fitter. The state computation needs to be redesigned fully
>    for bigjoiner. Semi-related to the aforementioned scaler work.
> - the modeset sequence is still a huge mess. That will have
>    to be the next major refactoring target I think.
>
> Pushed the lot here:
> git://github.com/vsyrjala/linux.git pipesrc_rect_3
>
> Ville Syrjälä (12):
>    drm/i915: Fix cursor coordinates on bigjoiner slave
>    drm/i915: Remove nop bigjoiner state copy
>    drm/i915: Rename variables in intel_crtc_compute_config()
>    drm/i915: Extract intel_splitter_adjust_timings()
>    drm/i915: Extract intel_bigjoiner_adjust_timings()
>    drm/i915: Extract intel_crtc_compute_pipe_src()
>    drm/i915: Extract intel_crtc_compute_pipe_mode()
>    drm/i915: Fix MSO vs. bigjoiner timings confusion
>    drm/i915: Start tracking PIPESRC as a drm_rect
>    drm/i915: Eliminate bigjoiner boolean
>    drm/i915: Use bigjoiner_pipes more
>    drm/i915: Make the PIPESC rect relative to the entire bigjoiner area
>
>   .../gpu/drm/i915/display/intel_atomic_plane.c |  20 +-
>   drivers/gpu/drm/i915/display/intel_cursor.c   |   7 +-
>   drivers/gpu/drm/i915/display/intel_display.c  | 350 +++++++++++-------
>   .../drm/i915/display/intel_display_debugfs.c  |   6 +-
>   .../drm/i915/display/intel_display_types.h    |   5 +-
>   drivers/gpu/drm/i915/display/intel_dp.c       |  13 +-
>   drivers/gpu/drm/i915/display/intel_overlay.c  |  22 +-
>   drivers/gpu/drm/i915/display/intel_panel.c    |  70 ++--
>   drivers/gpu/drm/i915/display/intel_vdsc.c     |   8 +-
>   drivers/gpu/drm/i915/display/skl_scaler.c     |  12 +-
>   .../drm/i915/display/skl_universal_plane.c    |   4 +-
>   11 files changed, 294 insertions(+), 223 deletions(-)
>