[Intel-gfx] [PATCH v2 0/8] drm/i915: pfit/scaler rework prep stuff

[Intel-gfx] [PATCH v2 0/8] drm/i915: pfit/scaler rework prep stuff

[Ville Syrjala]

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

Rebased pfit/scaler rework prep stuff. The eventual aim is to expose
margin properties for external displays. Main use of which is to
squish the image down a bit to avoid overscan on displays that insist
on always overscanning. That will happen via the pfit/pipe scaler.

And to avoid the user getting a black/corrupted screen we also need
to add proper checks against various hw limits (scaling factors,
pfit window coordinates, etc.). In fact we're already missing a bunch
of checks for skl+ plane scaling cases (eg. the scaler simply doesn't
work correctly with >4k resolutions so thos have to be rejected).

Anyways, it's a fair amount of work so I'm posting it in smaller
chunks.

The entire work can be found here:
git://github.com/vsyrjala/linux.git scaler_rework_2

Ville Syrjälä (8):
  drm/i915: Parametrize PFIT_PIPE
  drm/i915: Use intel_de_write_fw() for skl+ scaler registers
  drm/i915: Fix skl+ non-scaled pfit modes
  drm/i915: Flatten a bunch of the pfit functions
  drm/i915: Use drm_rect to store the pfit window pos/size
  drm/i915: s/pipe_config/crtc_state/ in pfit functions
  drm/i915: Pass connector state to pfit calculations
  drm/i915: Have pfit calculations return an error code

 drivers/gpu/drm/i915/display/icl_dsi.c        |  11 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 319 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_display.h  |   1 -
 .../drm/i915/display/intel_display_types.h    |   3 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  38 +--
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  37 +-
 drivers/gpu/drm/i915/display/intel_lvds.c     |  16 +-
 drivers/gpu/drm/i915/display/intel_panel.c    | 127 +++----
 drivers/gpu/drm/i915/display/intel_panel.h    |  10 +-
 drivers/gpu/drm/i915/display/vlv_dsi.c        |   9 +-
 drivers/gpu/drm/i915/i915_reg.h               |   1 +
 11 files changed, 288 insertions(+), 284 deletions(-)

-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 1/8] drm/i915: Parametrize PFIT_PIPE

[Ville Syrjala]

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

Make the PFIT_PIPE stuff less ugly via parametrization.

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

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index cba2f1c2557f..8b0730f4c442 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -434,8 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 	/* 965+ wants fuzzy fitting */
 	/* FIXME: handle multiple panels by failing gracefully */
 	if (INTEL_GEN(dev_priv) >= 4)
-		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
-				 PFIT_FILTER_FUZZY);
+		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
 
 out:
 	if ((pfit_control & PFIT_ENABLE) == 0) {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b09c1d6dc0aa..faf8945a51b0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4928,6 +4928,7 @@ enum {
 #define   PFIT_ENABLE		(1 << 31)
 #define   PFIT_PIPE_MASK	(3 << 29)
 #define   PFIT_PIPE_SHIFT	29
+#define   PFIT_PIPE(pipe)	((pipe) << 29)
 #define   VERT_INTERP_DISABLE	(0 << 10)
 #define   VERT_INTERP_BILINEAR	(1 << 10)
 #define   VERT_INTERP_MASK	(3 << 10)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Parametrize PFIT_PIPE

[Jani Nikula]

On Wed, 12 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the PFIT_PIPE stuff less ugly via parametrization.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 3 +--
>  drivers/gpu/drm/i915/i915_reg.h            | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index cba2f1c2557f..8b0730f4c442 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -434,8 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  	/* 965+ wants fuzzy fitting */
>  	/* FIXME: handle multiple panels by failing gracefully */
>  	if (INTEL_GEN(dev_priv) >= 4)
> -		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
> -				 PFIT_FILTER_FUZZY);
> +		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
>  
>  out:
>  	if ((pfit_control & PFIT_ENABLE) == 0) {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b09c1d6dc0aa..faf8945a51b0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4928,6 +4928,7 @@ enum {
>  #define   PFIT_ENABLE		(1 << 31)
>  #define   PFIT_PIPE_MASK	(3 << 29)
>  #define   PFIT_PIPE_SHIFT	29
> +#define   PFIT_PIPE(pipe)	((pipe) << 29)

This is fine, but might have as well defined this in terms of
REG_FIELD_PREP. I especially like it for parametrized stuff because it
ensures we don't flood the value outside the field.

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

>  #define   VERT_INTERP_DISABLE	(0 << 10)
>  #define   VERT_INTERP_BILINEAR	(1 << 10)
>  #define   VERT_INTERP_MASK	(3 << 10)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Parametrize PFIT_PIPE

[Ville Syrjälä]

On Wed, Feb 12, 2020 at 07:43:51PM +0200, Jani Nikula wrote:
> On Wed, 12 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make the PFIT_PIPE stuff less ugly via parametrization.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_panel.c | 3 +--
> >  drivers/gpu/drm/i915/i915_reg.h            | 1 +
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> > index cba2f1c2557f..8b0730f4c442 100644
> > --- a/drivers/gpu/drm/i915/display/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> > @@ -434,8 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> >  	/* 965+ wants fuzzy fitting */
> >  	/* FIXME: handle multiple panels by failing gracefully */
> >  	if (INTEL_GEN(dev_priv) >= 4)
> > -		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
> > -				 PFIT_FILTER_FUZZY);
> > +		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
> >  
> >  out:
> >  	if ((pfit_control & PFIT_ENABLE) == 0) {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index b09c1d6dc0aa..faf8945a51b0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4928,6 +4928,7 @@ enum {
> >  #define   PFIT_ENABLE		(1 << 31)
> >  #define   PFIT_PIPE_MASK	(3 << 29)
> >  #define   PFIT_PIPE_SHIFT	29
> > +#define   PFIT_PIPE(pipe)	((pipe) << 29)
> 
> This is fine, but might have as well defined this in terms of
> REG_FIELD_PREP. I especially like it for parametrized stuff because it
> ensures we don't flood the value outside the field.

Old patch, old tricks.

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> >  #define   VERT_INTERP_DISABLE	(0 << 10)
> >  #define   VERT_INTERP_BILINEAR	(1 << 10)
> >  #define   VERT_INTERP_MASK	(3 << 10)
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Parametrize PFIT_PIPE

[Manasi Navare]

On Wed, Feb 12, 2020 at 07:43:51PM +0200, Jani Nikula wrote:
> On Wed, 12 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make the PFIT_PIPE stuff less ugly via parametrization.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_panel.c | 3 +--
> >  drivers/gpu/drm/i915/i915_reg.h            | 1 +
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> > index cba2f1c2557f..8b0730f4c442 100644
> > --- a/drivers/gpu/drm/i915/display/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> > @@ -434,8 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> >  	/* 965+ wants fuzzy fitting */
> >  	/* FIXME: handle multiple panels by failing gracefully */
> >  	if (INTEL_GEN(dev_priv) >= 4)
> > -		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
> > -				 PFIT_FILTER_FUZZY);
> > +		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
> >  
> >  out:
> >  	if ((pfit_control & PFIT_ENABLE) == 0) {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index b09c1d6dc0aa..faf8945a51b0 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4928,6 +4928,7 @@ enum {
> >  #define   PFIT_ENABLE		(1 << 31)
> >  #define   PFIT_PIPE_MASK	(3 << 29)
> >  #define   PFIT_PIPE_SHIFT	29
> > +#define   PFIT_PIPE(pipe)	((pipe) << 29)
> 
> This is fine, but might have as well defined this in terms of
> REG_FIELD_PREP. I especially like it for parametrized stuff because it
> ensures we don't flood the value outside the field.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Was just reviewing this series and noticed that Jani had suggested using
REG_FIELD_PREP stuff here, are you going to change that Ville?

Looks good otherwise

Manasi
> 
> >  #define   VERT_INTERP_DISABLE	(0 << 10)
> >  #define   VERT_INTERP_BILINEAR	(1 << 10)
> >  #define   VERT_INTERP_MASK	(3 << 10)
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Parametrize PFIT_PIPE

[Ville Syrjälä]

On Wed, Apr 01, 2020 at 03:48:26PM -0700, Manasi Navare wrote:
> On Wed, Feb 12, 2020 at 07:43:51PM +0200, Jani Nikula wrote:
> > On Wed, 12 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Make the PFIT_PIPE stuff less ugly via parametrization.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_panel.c | 3 +--
> > >  drivers/gpu/drm/i915/i915_reg.h            | 1 +
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> > > index cba2f1c2557f..8b0730f4c442 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> > > @@ -434,8 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > >  	/* 965+ wants fuzzy fitting */
> > >  	/* FIXME: handle multiple panels by failing gracefully */
> > >  	if (INTEL_GEN(dev_priv) >= 4)
> > > -		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) |
> > > -				 PFIT_FILTER_FUZZY);
> > > +		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
> > >  
> > >  out:
> > >  	if ((pfit_control & PFIT_ENABLE) == 0) {
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index b09c1d6dc0aa..faf8945a51b0 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4928,6 +4928,7 @@ enum {
> > >  #define   PFIT_ENABLE		(1 << 31)
> > >  #define   PFIT_PIPE_MASK	(3 << 29)
> > >  #define   PFIT_PIPE_SHIFT	29
> > > +#define   PFIT_PIPE(pipe)	((pipe) << 29)
> > 
> > This is fine, but might have as well defined this in terms of
> > REG_FIELD_PREP. I especially like it for parametrized stuff because it
> > ensures we don't flood the value outside the field.
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> Was just reviewing this series and noticed that Jani had suggested using
> REG_FIELD_PREP stuff here, are you going to change that Ville?

IIRC I already pushed this.

> 
> Looks good otherwise
> 
> Manasi
> > 
> > >  #define   VERT_INTERP_DISABLE	(0 << 10)
> > >  #define   VERT_INTERP_BILINEAR	(1 << 10)
> > >  #define   VERT_INTERP_MASK	(3 << 10)
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 2/8] drm/i915: Use intel_de_write_fw() for skl+ scaler registers

[Ville Syrjala]

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

We have to write quite a few registers when programming the
pipe scaler. Let's use intel_de_write_fw() for these to reduce
the lockdep overhead a bit. All plane registers (including plane
scaler) already do this.

We already had a few accidental intel_de_write_fw() in there.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 61ba1f2256a0..de50aa0b076c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4494,10 +4494,15 @@ static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	intel_de_write(dev_priv, SKL_PS_CTRL(intel_crtc->pipe, id), 0);
-	intel_de_write(dev_priv, SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
-	intel_de_write(dev_priv, SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
+	intel_de_write_fw(dev_priv, SKL_PS_CTRL(intel_crtc->pipe, id), 0);
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 /*
@@ -6234,6 +6239,7 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->pch_pfit.enabled) {
 		u16 uv_rgb_hphase, uv_rgb_vphase;
 		int pfit_w, pfit_h, hscale, vscale;
+		unsigned long irqflags;
 		int id;
 
 		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
@@ -6249,16 +6255,21 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
 
 		id = scaler_state->scaler_id;
-		intel_de_write(dev_priv, SKL_PS_CTRL(pipe, id),
-			       PS_SCALER_EN | PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
+
+		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
+				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
 		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
 				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
 		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
 				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
-		intel_de_write(dev_priv, SKL_PS_WIN_POS(pipe, id),
-			       crtc_state->pch_pfit.pos);
-		intel_de_write(dev_priv, SKL_PS_WIN_SZ(pipe, id),
-			       crtc_state->pch_pfit.size);
+		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
+				  crtc_state->pch_pfit.pos);
+		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
+				  crtc_state->pch_pfit.size);
+
+		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 	}
 }
 
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/8] drm/i915: Use intel_de_write_fw() for skl+ scaler registers

[Jani Nikula]

On Wed, 12 Feb 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have to write quite a few registers when programming the
> pipe scaler. Let's use intel_de_write_fw() for these to reduce
> the lockdep overhead a bit. All plane registers (including plane
> scaler) already do this.
>
> We already had a few accidental intel_de_write_fw() in there.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 29 ++++++++++++++------
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 61ba1f2256a0..de50aa0b076c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4494,10 +4494,15 @@ static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);

Hrmh, I don't like how the uncore lock leaks through the intel_de_*
abstractions. But I guess I dislike adding wrappers for spin locks even
less.

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


>  
> -	intel_de_write(dev_priv, SKL_PS_CTRL(intel_crtc->pipe, id), 0);
> -	intel_de_write(dev_priv, SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
> -	intel_de_write(dev_priv, SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
> +	intel_de_write_fw(dev_priv, SKL_PS_CTRL(intel_crtc->pipe, id), 0);
> +	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(intel_crtc->pipe, id), 0);
> +	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(intel_crtc->pipe, id), 0);
> +
> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
>  /*
> @@ -6234,6 +6239,7 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	if (crtc_state->pch_pfit.enabled) {
>  		u16 uv_rgb_hphase, uv_rgb_vphase;
>  		int pfit_w, pfit_h, hscale, vscale;
> +		unsigned long irqflags;
>  		int id;
>  
>  		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> @@ -6249,16 +6255,21 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
>  
>  		id = scaler_state->scaler_id;
> -		intel_de_write(dev_priv, SKL_PS_CTRL(pipe, id),
> -			       PS_SCALER_EN | PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> +
> +		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +
> +		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> +				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
>  		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
>  				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
>  		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
>  				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> -		intel_de_write(dev_priv, SKL_PS_WIN_POS(pipe, id),
> -			       crtc_state->pch_pfit.pos);
> -		intel_de_write(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> -			       crtc_state->pch_pfit.size);
> +		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> +				  crtc_state->pch_pfit.pos);
> +		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> +				  crtc_state->pch_pfit.size);
> +
> +		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  	}
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 3/8] drm/i915: Fix skl+ non-scaled pfit modes

[Ville Syrjala]

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

Fix skl_update_scaler_crtc() to deal with different scaling
modes correctly. The current implementation assumes
DRM_MODE_SCALE_FULLSCREEN. Fortunately we don't expose any
border properties currently so the code does actually end
up doing the right thing (assigning a scaler for pfit).
The code does need to be fixed before any borders are
exposed.

Also we have redundant calls to skl_update_scaler_crtc() in
dp/hdmi .compute_config() which can be nuked. They were anyway
called before we had even computed the pfit state so were
basically nonsense. The real call we need to keep is in
intel_crtc_atomic_check().

v2: Deal witrh skl_update_scaler_crtc() in intel_dp_ycbcr420_config()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 40 ++++++++++----------
 drivers/gpu/drm/i915/display/intel_display.h |  1 -
 drivers/gpu/drm/i915/display/intel_dp.c      | 15 --------
 drivers/gpu/drm/i915/display/intel_hdmi.c    |  6 ---
 4 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index de50aa0b076c..becc6322b7dc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6101,30 +6101,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	return 0;
 }
 
-/**
- * skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
- *
- * @state: crtc's scaler state
- *
- * Return
- *     0 - scaler_usage updated successfully
- *    error - requested scaling cannot be supported or other error condition
- */
-int skl_update_scaler_crtc(struct intel_crtc_state *state)
+static int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state)
 {
-	const struct drm_display_mode *adjusted_mode = &state->hw.adjusted_mode;
-	bool need_scaler = false;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
+	int width, height;
 
-	if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-	    state->pch_pfit.enabled)
-		need_scaler = true;
+	if (crtc_state->pch_pfit.enabled) {
+		u32 pfit_size = crtc_state->pch_pfit.size;
+
+		width = pfit_size >> 16;
+		height = pfit_size & 0xffff;
+	} else {
+		width = adjusted_mode->crtc_hdisplay;
+		height = adjusted_mode->crtc_vdisplay;
+	}
 
-	return skl_update_scaler(state, !state->hw.active, SKL_CRTC_INDEX,
-				 &state->scaler_state.scaler_id,
-				 state->pipe_src_w, state->pipe_src_h,
-				 adjusted_mode->crtc_hdisplay,
-				 adjusted_mode->crtc_vdisplay, NULL, 0,
-				 need_scaler);
+	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,
+				 width, height, NULL, 0,
+				 crtc_state->pch_pfit.enabled);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 75438a136d58..6291d3dbc513 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -584,7 +584,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
 				  struct intel_crtc_state *crtc_state);
 
 u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
-int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
 void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
 u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f4dede6253f8..a827eac8acc2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2307,7 +2307,6 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	int ret;
 
 	if (!drm_mode_is_420_only(info, adjusted_mode) ||
 	    !intel_dp_get_colorimetry_status(intel_dp) ||
@@ -2316,13 +2315,6 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	/* YCBCR 420 output conversion needs a scaler */
-	ret = skl_update_scaler_crtc(crtc_state);
-	if (ret) {
-		DRM_DEBUG_KMS("Scaler allocation for output failed\n");
-		return ret;
-	}
-
 	intel_pch_panel_fitting(crtc, crtc_state, DRM_MODE_SCALE_FULLSCREEN);
 
 	return 0;
@@ -2400,7 +2392,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	else
 		ret = intel_dp_ycbcr420_config(intel_dp, &intel_connector->base,
 					       pipe_config);
-
 	if (ret)
 		return ret;
 
@@ -2416,12 +2407,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
 				       adjusted_mode);
 
-		if (INTEL_GEN(dev_priv) >= 9) {
-			ret = skl_update_scaler_crtc(pipe_config);
-			if (ret)
-				return ret;
-		}
-
 		if (HAS_GMCH(dev_priv))
 			intel_gmch_panel_fitting(intel_crtc, pipe_config,
 						 conn_state->scaling_mode);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index e68bafb76cb1..1e42b01045b1 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2308,12 +2308,6 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 
 	config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	/* YCBCR 420 output conversion needs a scaler */
-	if (skl_update_scaler_crtc(config)) {
-		DRM_DEBUG_KMS("Scaler allocation for output failed\n");
-		return false;
-	}
-
 	intel_pch_panel_fitting(intel_crtc, config,
 				DRM_MODE_SCALE_FULLSCREEN);
 
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/8] drm/i915: Fix skl+ non-scaled pfit modes

[Manasi Navare]

On Wed, Feb 12, 2020 at 06:17:33PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fix skl_update_scaler_crtc() to deal with different scaling
> modes correctly. The current implementation assumes
> DRM_MODE_SCALE_FULLSCREEN. Fortunately we don't expose any
> border properties currently so the code does actually end
> up doing the right thing (assigning a scaler for pfit).
> The code does need to be fixed before any borders are
> exposed.
> 
> Also we have redundant calls to skl_update_scaler_crtc() in
> dp/hdmi .compute_config() which can be nuked. They were anyway
> called before we had even computed the pfit state so were
> basically nonsense. The real call we need to keep is in
> intel_crtc_atomic_check().
> 
> v2: Deal witrh skl_update_scaler_crtc() in intel_dp_ycbcr420_config()
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just keeping the call in intel_crtc_atomic_check() looks good

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

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 40 ++++++++++----------
>  drivers/gpu/drm/i915/display/intel_display.h |  1 -
>  drivers/gpu/drm/i915/display/intel_dp.c      | 15 --------
>  drivers/gpu/drm/i915/display/intel_hdmi.c    |  6 ---
>  4 files changed, 19 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index de50aa0b076c..becc6322b7dc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6101,30 +6101,28 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  	return 0;
>  }
>  
> -/**
> - * skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
> - *
> - * @state: crtc's scaler state
> - *
> - * Return
> - *     0 - scaler_usage updated successfully
> - *    error - requested scaling cannot be supported or other error condition
> - */
> -int skl_update_scaler_crtc(struct intel_crtc_state *state)
> +static int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state)
>  {
> -	const struct drm_display_mode *adjusted_mode = &state->hw.adjusted_mode;
> -	bool need_scaler = false;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->hw.adjusted_mode;
> +	int width, height;
>  
> -	if (state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> -	    state->pch_pfit.enabled)
> -		need_scaler = true;
> +	if (crtc_state->pch_pfit.enabled) {
> +		u32 pfit_size = crtc_state->pch_pfit.size;
> +
> +		width = pfit_size >> 16;
> +		height = pfit_size & 0xffff;
> +	} else {
> +		width = adjusted_mode->crtc_hdisplay;
> +		height = adjusted_mode->crtc_vdisplay;
> +	}
>  
> -	return skl_update_scaler(state, !state->hw.active, SKL_CRTC_INDEX,
> -				 &state->scaler_state.scaler_id,
> -				 state->pipe_src_w, state->pipe_src_h,
> -				 adjusted_mode->crtc_hdisplay,
> -				 adjusted_mode->crtc_vdisplay, NULL, 0,
> -				 need_scaler);
> +	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,
> +				 width, height, NULL, 0,
> +				 crtc_state->pch_pfit.enabled);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 75438a136d58..6291d3dbc513 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -584,7 +584,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
>  				  struct intel_crtc_state *crtc_state);
>  
>  u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
> -int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>  void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
>  void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
>  u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index f4dede6253f8..a827eac8acc2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2307,7 +2307,6 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	int ret;
>  
>  	if (!drm_mode_is_420_only(info, adjusted_mode) ||
>  	    !intel_dp_get_colorimetry_status(intel_dp) ||
> @@ -2316,13 +2315,6 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
>  
>  	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>  
> -	/* YCBCR 420 output conversion needs a scaler */
> -	ret = skl_update_scaler_crtc(crtc_state);
> -	if (ret) {
> -		DRM_DEBUG_KMS("Scaler allocation for output failed\n");
> -		return ret;
> -	}
> -
>  	intel_pch_panel_fitting(crtc, crtc_state, DRM_MODE_SCALE_FULLSCREEN);
>  
>  	return 0;
> @@ -2400,7 +2392,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	else
>  		ret = intel_dp_ycbcr420_config(intel_dp, &intel_connector->base,
>  					       pipe_config);
> -
>  	if (ret)
>  		return ret;
>  
> @@ -2416,12 +2407,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>  				       adjusted_mode);
>  
> -		if (INTEL_GEN(dev_priv) >= 9) {
> -			ret = skl_update_scaler_crtc(pipe_config);
> -			if (ret)
> -				return ret;
> -		}
> -
>  		if (HAS_GMCH(dev_priv))
>  			intel_gmch_panel_fitting(intel_crtc, pipe_config,
>  						 conn_state->scaling_mode);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index e68bafb76cb1..1e42b01045b1 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2308,12 +2308,6 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
>  
>  	config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>  
> -	/* YCBCR 420 output conversion needs a scaler */
> -	if (skl_update_scaler_crtc(config)) {
> -		DRM_DEBUG_KMS("Scaler allocation for output failed\n");
> -		return false;
> -	}
> -
>  	intel_pch_panel_fitting(intel_crtc, config,
>  				DRM_MODE_SCALE_FULLSCREEN);
>  
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/8] drm/i915: Flatten a bunch of the pfit functions

[Ville Syrjala]

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

Most of the pfit functions are of the form:

func()
{
	if (pfit_enabled) {
		...
	}
}

Flip the pfit_enabled check around to flatten the functions.

And while we're touching all this let's do the usual
s/pipe_config/crtc_state/ replacement.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index becc6322b7dc..796e27c4aece 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6233,42 +6233,42 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
+	u16 uv_rgb_hphase, uv_rgb_vphase;
+	int pfit_w, pfit_h, hscale, vscale;
+	unsigned long irqflags;
+	int id;
 
-	if (crtc_state->pch_pfit.enabled) {
-		u16 uv_rgb_hphase, uv_rgb_vphase;
-		int pfit_w, pfit_h, hscale, vscale;
-		unsigned long irqflags;
-		int id;
+	if (!crtc_state->pch_pfit.enabled)
+		return;
 
-		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
-			return;
+	if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
+		return;
 
-		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
-		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
+	pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
+	pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
 
-		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
-		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
+	hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
+	vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
 
-		uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
-		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
+	uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
+	uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
 
-		id = scaler_state->scaler_id;
+	id = scaler_state->scaler_id;
 
-		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
-				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
-		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
-				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
-		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
-				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
-		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
-				  crtc_state->pch_pfit.pos);
-		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
-				  crtc_state->pch_pfit.size);
+	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
+			  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
+	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
+			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
+	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
+			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
+			  crtc_state->pch_pfit.pos);
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
+			  crtc_state->pch_pfit.size);
 
-		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-	}
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
@@ -6277,22 +6277,23 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
-	if (crtc_state->pch_pfit.enabled) {
-		/* Force use of hard-coded filter coefficients
-		 * as some pre-programmed values are broken,
-		 * e.g. x201.
-		 */
-		if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
-			intel_de_write(dev_priv, PF_CTL(pipe),
-				       PF_ENABLE | PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
-		else
-			intel_de_write(dev_priv, PF_CTL(pipe),
-				       PF_ENABLE | PF_FILTER_MED_3x3);
-		intel_de_write(dev_priv, PF_WIN_POS(pipe),
-			       crtc_state->pch_pfit.pos);
-		intel_de_write(dev_priv, PF_WIN_SZ(pipe),
-			       crtc_state->pch_pfit.size);
-	}
+	if (!crtc_state->pch_pfit.enabled)
+		return;
+
+	/* Force use of hard-coded filter coefficients
+	 * as some pre-programmed values are broken,
+	 * e.g. x201.
+	 */
+	if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
+		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
+			       PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
+	else
+		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
+			       PF_FILTER_MED_3x3);
+	intel_de_write(dev_priv, PF_WIN_POS(pipe),
+		       crtc_state->pch_pfit.pos);
+	intel_de_write(dev_priv, PF_WIN_SZ(pipe),
+		       crtc_state->pch_pfit.size);
 }
 
 void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
@@ -7107,11 +7108,12 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
 
 	/* To avoid upsetting the power well on haswell only disable the pfit if
 	 * it's in use. The hw state code will make sure we get this right. */
-	if (old_crtc_state->pch_pfit.enabled) {
-		intel_de_write(dev_priv, PF_CTL(pipe), 0);
-		intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
-		intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
-	}
+	if (!old_crtc_state->pch_pfit.enabled)
+		return;
+
+	intel_de_write(dev_priv, PF_CTL(pipe), 0);
+	intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
+	intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
 }
 
 static void ilk_crtc_disable(struct intel_atomic_state *state,
@@ -7927,39 +7929,35 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
 		(crtc->pipe == PIPE_A || IS_I915G(dev_priv));
 }
 
-static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
+static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
 {
-	u32 pixel_rate;
-
-	pixel_rate = pipe_config->hw.adjusted_mode.crtc_clock;
+	u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
+	u32 pfit_size = crtc_state->pch_pfit.size;
+	u64 pipe_w, pipe_h, pfit_w, pfit_h;
 
 	/*
 	 * We only use IF-ID interlacing. If we ever use
 	 * PF-ID we'll need to adjust the pixel_rate here.
 	 */
 
-	if (pipe_config->pch_pfit.enabled) {
-		u64 pipe_w, pipe_h, pfit_w, pfit_h;
-		u32 pfit_size = pipe_config->pch_pfit.size;
-
-		pipe_w = pipe_config->pipe_src_w;
-		pipe_h = pipe_config->pipe_src_h;
+	if (!crtc_state->pch_pfit.enabled)
+		return pixel_rate;
 
-		pfit_w = (pfit_size >> 16) & 0xFFFF;
-		pfit_h = pfit_size & 0xFFFF;
-		if (pipe_w < pfit_w)
-			pipe_w = pfit_w;
-		if (pipe_h < pfit_h)
-			pipe_h = pfit_h;
+	pipe_w = crtc_state->pipe_src_w;
+	pipe_h = crtc_state->pipe_src_h;
 
-		if (WARN_ON(!pfit_w || !pfit_h))
-			return pixel_rate;
+	pfit_w = (pfit_size >> 16) & 0xFFFF;
+	pfit_h = pfit_size & 0xFFFF;
+	if (pipe_w < pfit_w)
+		pipe_w = pfit_w;
+	if (pipe_h < pfit_h)
+		pipe_h = pfit_h;
 
-		pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
-				     pfit_w * pfit_h);
-	}
+	if (WARN_ON(!pfit_w || !pfit_h))
+		return pixel_rate;
 
-	return pixel_rate;
+	return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
+		       pfit_w * pfit_h);
 }
 
 static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
@@ -9153,9 +9151,9 @@ static bool i9xx_has_pfit(struct drm_i915_private *dev_priv)
 		IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
 }
 
-static void i9xx_get_pfit_config(struct intel_crtc *crtc,
-				 struct intel_crtc_state *pipe_config)
+static void i9xx_get_pfit_config(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 tmp;
 
@@ -9175,9 +9173,9 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
 			return;
 	}
 
-	pipe_config->gmch_pfit.control = tmp;
-	pipe_config->gmch_pfit.pgm_ratios = intel_de_read(dev_priv,
-							  PFIT_PGM_RATIOS);
+	crtc_state->gmch_pfit.control = tmp;
+	crtc_state->gmch_pfit.pgm_ratios =
+		intel_de_read(dev_priv, PFIT_PGM_RATIOS);
 }
 
 static void vlv_crtc_clock_get(struct intel_crtc *crtc,
@@ -9427,7 +9425,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	intel_get_pipe_timings(crtc, pipe_config);
 	intel_get_pipe_src_size(crtc, pipe_config);
 
-	i9xx_get_pfit_config(crtc, pipe_config);
+	i9xx_get_pfit_config(pipe_config);
 
 	if (INTEL_GEN(dev_priv) >= 4) {
 		/* No way to read it out on pipes B and C */
@@ -10393,37 +10391,37 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
 				     &pipe_config->fdi_m_n, NULL);
 }
 
-static void skl_get_pfit_config(struct intel_crtc *crtc,
-				struct intel_crtc_state *pipe_config)
+static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc_scaler_state *scaler_state = &pipe_config->scaler_state;
-	u32 ps_ctrl = 0;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
 	int id = -1;
 	int i;
 
 	/* find scaler attached to this pipe */
 	for (i = 0; i < crtc->num_scalers; i++) {
-		ps_ctrl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
-		if (ps_ctrl & PS_SCALER_EN && !(ps_ctrl & PS_PLANE_SEL_MASK)) {
-			id = i;
-			pipe_config->pch_pfit.enabled = true;
-			pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
-								  SKL_PS_WIN_POS(crtc->pipe, i));
-			pipe_config->pch_pfit.size = intel_de_read(dev_priv,
-								   SKL_PS_WIN_SZ(crtc->pipe, i));
-			scaler_state->scalers[i].in_use = true;
-			break;
-		}
+		u32 tmp;
+
+		tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
+		if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
+			continue;
+
+		id = i;
+		crtc_state->pch_pfit.enabled = true;
+		crtc_state->pch_pfit.pos =
+			intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
+		crtc_state->pch_pfit.size =
+			intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
+		scaler_state->scalers[i].in_use = true;
+		break;
 	}
 
 	scaler_state->scaler_id = id;
-	if (id >= 0) {
+	if (id >= 0)
 		scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
-	} else {
+	else
 		scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
-	}
 }
 
 static void
@@ -10559,30 +10557,29 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
 	kfree(intel_fb);
 }
 
-static void ilk_get_pfit_config(struct intel_crtc *crtc,
-				struct intel_crtc_state *pipe_config)
+static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	u32 tmp;
 
 	tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
+	if ((tmp & PF_ENABLE) == 0)
+		return;
 
-	if (tmp & PF_ENABLE) {
-		pipe_config->pch_pfit.enabled = true;
-		pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
-							  PF_WIN_POS(crtc->pipe));
-		pipe_config->pch_pfit.size = intel_de_read(dev_priv,
-							   PF_WIN_SZ(crtc->pipe));
-
-		/* We currently do not free assignements of panel fitters on
-		 * ivb/hsw (since we don't use the higher upscaling modes which
-		 * differentiates them) so just WARN about this case for now. */
-		if (IS_GEN(dev_priv, 7)) {
-			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
-				PF_PIPE_SEL_IVB(crtc->pipe));
-		}
-	}
+	crtc_state->pch_pfit.enabled = true;
+	crtc_state->pch_pfit.pos =
+		intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
+	crtc_state->pch_pfit.size =
+		intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
+
+	/*
+	 * We currently do not free assignements of panel fitters on
+	 * ivb/hsw (since we don't use the higher upscaling modes which
+	 * differentiates them) so just WARN about this case for now.
+	 */
+	WARN_ON(IS_GEN(dev_priv, 7) &&
+		(tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
 }
 
 static bool ilk_get_pipe_config(struct intel_crtc *crtc,
@@ -10694,7 +10691,7 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
 	intel_get_pipe_timings(crtc, pipe_config);
 	intel_get_pipe_src_size(crtc, pipe_config);
 
-	ilk_get_pfit_config(crtc, pipe_config);
+	ilk_get_pfit_config(pipe_config);
 
 	ret = true;
 
@@ -11219,9 +11216,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 		power_domain_mask |= BIT_ULL(power_domain);
 
 		if (INTEL_GEN(dev_priv) >= 9)
-			skl_get_pfit_config(crtc, pipe_config);
+			skl_get_pfit_config(pipe_config);
 		else
-			ilk_get_pfit_config(crtc, pipe_config);
+			ilk_get_pfit_config(pipe_config);
 	}
 
 	if (hsw_crtc_supports_ips(crtc)) {
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/8] drm/i915: Flatten a bunch of the pfit functions

[Manasi Navare]

On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Most of the pfit functions are of the form:
> 
> func()
> {
> 	if (pfit_enabled) {
> 		...
> 	}
> }
> 
> Flip the pfit_enabled check around to flatten the functions.
> 
> And while we're touching all this let's do the usual
> s/pipe_config/crtc_state/ replacement.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 233 +++++++++----------
>  1 file changed, 115 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index becc6322b7dc..796e27c4aece 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6233,42 +6233,42 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	enum pipe pipe = crtc->pipe;
>  	const struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> +	u16 uv_rgb_hphase, uv_rgb_vphase;
> +	int pfit_w, pfit_h, hscale, vscale;
> +	unsigned long irqflags;
> +	int id;
>  
> -	if (crtc_state->pch_pfit.enabled) {
> -		u16 uv_rgb_hphase, uv_rgb_vphase;
> -		int pfit_w, pfit_h, hscale, vscale;
> -		unsigned long irqflags;
> -		int id;
> +	if (!crtc_state->pch_pfit.enabled)
> +		return;
>  
> -		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> -			return;
> +	if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> +		return;
>  
> -		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> -		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> +	pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> +	pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
>  
> -		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> -		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> +	hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> +	vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
>  
> -		uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> -		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> +	uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> +	uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
>  
> -		id = scaler_state->scaler_id;
> +	id = scaler_state->scaler_id;
>  
> -		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> -				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> -		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> -		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> -		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> -				  crtc_state->pch_pfit.pos);
> -		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> -				  crtc_state->pch_pfit.size);
> +	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> +			  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> +	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> +	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> +	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> +			  crtc_state->pch_pfit.pos);
> +	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> +			  crtc_state->pch_pfit.size);
>  
> -		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -	}
> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
>  static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> @@ -6277,22 +6277,23 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
>  
> -	if (crtc_state->pch_pfit.enabled) {
> -		/* Force use of hard-coded filter coefficients
> -		 * as some pre-programmed values are broken,
> -		 * e.g. x201.
> -		 */
> -		if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> -			intel_de_write(dev_priv, PF_CTL(pipe),
> -				       PF_ENABLE | PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> -		else
> -			intel_de_write(dev_priv, PF_CTL(pipe),
> -				       PF_ENABLE | PF_FILTER_MED_3x3);
> -		intel_de_write(dev_priv, PF_WIN_POS(pipe),
> -			       crtc_state->pch_pfit.pos);
> -		intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> -			       crtc_state->pch_pfit.size);

Why dont we use the intel_de_write_fw() everywhere?

Manasi

> -	}
> +	if (!crtc_state->pch_pfit.enabled)
> +		return;
> +
> +	/* Force use of hard-coded filter coefficients
> +	 * as some pre-programmed values are broken,
> +	 * e.g. x201.
> +	 */
> +	if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> +			       PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> +	else
> +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> +			       PF_FILTER_MED_3x3);
> +	intel_de_write(dev_priv, PF_WIN_POS(pipe),
> +		       crtc_state->pch_pfit.pos);
> +	intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> +		       crtc_state->pch_pfit.size);
>  }
>  
>  void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
> @@ -7107,11 +7108,12 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
>  
>  	/* To avoid upsetting the power well on haswell only disable the pfit if
>  	 * it's in use. The hw state code will make sure we get this right. */
> -	if (old_crtc_state->pch_pfit.enabled) {
> -		intel_de_write(dev_priv, PF_CTL(pipe), 0);
> -		intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> -		intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> -	}
> +	if (!old_crtc_state->pch_pfit.enabled)
> +		return;
> +
> +	intel_de_write(dev_priv, PF_CTL(pipe), 0);
> +	intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> +	intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
>  }
>  
>  static void ilk_crtc_disable(struct intel_atomic_state *state,
> @@ -7927,39 +7929,35 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
>  		(crtc->pipe == PIPE_A || IS_I915G(dev_priv));
>  }
>  
> -static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
> +static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
>  {
> -	u32 pixel_rate;
> -
> -	pixel_rate = pipe_config->hw.adjusted_mode.crtc_clock;
> +	u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
> +	u32 pfit_size = crtc_state->pch_pfit.size;
> +	u64 pipe_w, pipe_h, pfit_w, pfit_h;
>  
>  	/*
>  	 * We only use IF-ID interlacing. If we ever use
>  	 * PF-ID we'll need to adjust the pixel_rate here.
>  	 */
>  
> -	if (pipe_config->pch_pfit.enabled) {
> -		u64 pipe_w, pipe_h, pfit_w, pfit_h;
> -		u32 pfit_size = pipe_config->pch_pfit.size;
> -
> -		pipe_w = pipe_config->pipe_src_w;
> -		pipe_h = pipe_config->pipe_src_h;
> +	if (!crtc_state->pch_pfit.enabled)
> +		return pixel_rate;
>  
> -		pfit_w = (pfit_size >> 16) & 0xFFFF;
> -		pfit_h = pfit_size & 0xFFFF;
> -		if (pipe_w < pfit_w)
> -			pipe_w = pfit_w;
> -		if (pipe_h < pfit_h)
> -			pipe_h = pfit_h;
> +	pipe_w = crtc_state->pipe_src_w;
> +	pipe_h = crtc_state->pipe_src_h;
>  
> -		if (WARN_ON(!pfit_w || !pfit_h))
> -			return pixel_rate;
> +	pfit_w = (pfit_size >> 16) & 0xFFFF;
> +	pfit_h = pfit_size & 0xFFFF;
> +	if (pipe_w < pfit_w)
> +		pipe_w = pfit_w;
> +	if (pipe_h < pfit_h)
> +		pipe_h = pfit_h;
>  
> -		pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> -				     pfit_w * pfit_h);
> -	}
> +	if (WARN_ON(!pfit_w || !pfit_h))
> +		return pixel_rate;
>  
> -	return pixel_rate;
> +	return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> +		       pfit_w * pfit_h);
>  }
>  
>  static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> @@ -9153,9 +9151,9 @@ static bool i9xx_has_pfit(struct drm_i915_private *dev_priv)
>  		IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
>  }
>  
> -static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> -				 struct intel_crtc_state *pipe_config)
> +static void i9xx_get_pfit_config(struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	u32 tmp;
>  
> @@ -9175,9 +9173,9 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
>  			return;
>  	}
>  
> -	pipe_config->gmch_pfit.control = tmp;
> -	pipe_config->gmch_pfit.pgm_ratios = intel_de_read(dev_priv,
> -							  PFIT_PGM_RATIOS);
> +	crtc_state->gmch_pfit.control = tmp;
> +	crtc_state->gmch_pfit.pgm_ratios =
> +		intel_de_read(dev_priv, PFIT_PGM_RATIOS);
>  }
>  
>  static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> @@ -9427,7 +9425,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	intel_get_pipe_timings(crtc, pipe_config);
>  	intel_get_pipe_src_size(crtc, pipe_config);
>  
> -	i9xx_get_pfit_config(crtc, pipe_config);
> +	i9xx_get_pfit_config(pipe_config);
>  
>  	if (INTEL_GEN(dev_priv) >= 4) {
>  		/* No way to read it out on pipes B and C */
> @@ -10393,37 +10391,37 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
>  				     &pipe_config->fdi_m_n, NULL);
>  }
>  
> -static void skl_get_pfit_config(struct intel_crtc *crtc,
> -				struct intel_crtc_state *pipe_config)
> +static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc_scaler_state *scaler_state = &pipe_config->scaler_state;
> -	u32 ps_ctrl = 0;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
>  	int id = -1;
>  	int i;
>  
>  	/* find scaler attached to this pipe */
>  	for (i = 0; i < crtc->num_scalers; i++) {
> -		ps_ctrl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> -		if (ps_ctrl & PS_SCALER_EN && !(ps_ctrl & PS_PLANE_SEL_MASK)) {
> -			id = i;
> -			pipe_config->pch_pfit.enabled = true;
> -			pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> -								  SKL_PS_WIN_POS(crtc->pipe, i));
> -			pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> -								   SKL_PS_WIN_SZ(crtc->pipe, i));
> -			scaler_state->scalers[i].in_use = true;
> -			break;
> -		}
> +		u32 tmp;
> +
> +		tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> +		if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
> +			continue;
> +
> +		id = i;
> +		crtc_state->pch_pfit.enabled = true;
> +		crtc_state->pch_pfit.pos =
> +			intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
> +		crtc_state->pch_pfit.size =
> +			intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
> +		scaler_state->scalers[i].in_use = true;
> +		break;
>  	}
>  
>  	scaler_state->scaler_id = id;
> -	if (id >= 0) {
> +	if (id >= 0)
>  		scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
> -	} else {
> +	else
>  		scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
> -	}
>  }
>  
>  static void
> @@ -10559,30 +10557,29 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
>  	kfree(intel_fb);
>  }
>  
> -static void ilk_get_pfit_config(struct intel_crtc *crtc,
> -				struct intel_crtc_state *pipe_config)
> +static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	u32 tmp;
>  
>  	tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
> +	if ((tmp & PF_ENABLE) == 0)
> +		return;
>  
> -	if (tmp & PF_ENABLE) {
> -		pipe_config->pch_pfit.enabled = true;
> -		pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> -							  PF_WIN_POS(crtc->pipe));
> -		pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> -							   PF_WIN_SZ(crtc->pipe));
> -
> -		/* We currently do not free assignements of panel fitters on
> -		 * ivb/hsw (since we don't use the higher upscaling modes which
> -		 * differentiates them) so just WARN about this case for now. */
> -		if (IS_GEN(dev_priv, 7)) {
> -			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
> -				PF_PIPE_SEL_IVB(crtc->pipe));
> -		}
> -	}
> +	crtc_state->pch_pfit.enabled = true;
> +	crtc_state->pch_pfit.pos =
> +		intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
> +	crtc_state->pch_pfit.size =
> +		intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
> +
> +	/*
> +	 * We currently do not free assignements of panel fitters on
> +	 * ivb/hsw (since we don't use the higher upscaling modes which
> +	 * differentiates them) so just WARN about this case for now.
> +	 */
> +	WARN_ON(IS_GEN(dev_priv, 7) &&
> +		(tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
>  }
>  
>  static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> @@ -10694,7 +10691,7 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
>  	intel_get_pipe_timings(crtc, pipe_config);
>  	intel_get_pipe_src_size(crtc, pipe_config);
>  
> -	ilk_get_pfit_config(crtc, pipe_config);
> +	ilk_get_pfit_config(pipe_config);
>  
>  	ret = true;
>  
> @@ -11219,9 +11216,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
>  		power_domain_mask |= BIT_ULL(power_domain);
>  
>  		if (INTEL_GEN(dev_priv) >= 9)
> -			skl_get_pfit_config(crtc, pipe_config);
> +			skl_get_pfit_config(pipe_config);
>  		else
> -			ilk_get_pfit_config(crtc, pipe_config);
> +			ilk_get_pfit_config(pipe_config);
>  	}
>  
>  	if (hsw_crtc_supports_ips(crtc)) {
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/8] drm/i915: Flatten a bunch of the pfit functions

[Ville Syrjälä]

On Wed, Apr 01, 2020 at 04:53:23PM -0700, Manasi Navare wrote:
> On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Most of the pfit functions are of the form:
> > 
> > func()
> > {
> > 	if (pfit_enabled) {
> > 		...
> > 	}
> > }
> > 
> > Flip the pfit_enabled check around to flatten the functions.
> > 
> > And while we're touching all this let's do the usual
> > s/pipe_config/crtc_state/ replacement.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 233 +++++++++----------
> >  1 file changed, 115 insertions(+), 118 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index becc6322b7dc..796e27c4aece 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6233,42 +6233,42 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
> >  	enum pipe pipe = crtc->pipe;
> >  	const struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> > +	u16 uv_rgb_hphase, uv_rgb_vphase;
> > +	int pfit_w, pfit_h, hscale, vscale;
> > +	unsigned long irqflags;
> > +	int id;
> >  
> > -	if (crtc_state->pch_pfit.enabled) {
> > -		u16 uv_rgb_hphase, uv_rgb_vphase;
> > -		int pfit_w, pfit_h, hscale, vscale;
> > -		unsigned long irqflags;
> > -		int id;
> > +	if (!crtc_state->pch_pfit.enabled)
> > +		return;
> >  
> > -		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> > -			return;
> > +	if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> > +		return;
> >  
> > -		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> > -		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> > +	pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> > +	pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> >  
> > -		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > -		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> > +	hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > +	vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> >  
> > -		uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> > -		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> > +	uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> > +	uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> >  
> > -		id = scaler_state->scaler_id;
> > +	id = scaler_state->scaler_id;
> >  
> > -		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> > -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> > -				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> > -		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> > -		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> > -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> > -		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> > -				  crtc_state->pch_pfit.pos);
> > -		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> > -				  crtc_state->pch_pfit.size);
> > +	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> > +			  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> > +	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> > +	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> > +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> > +	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> > +			  crtc_state->pch_pfit.pos);
> > +	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> > +			  crtc_state->pch_pfit.size);
> >  
> > -		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > -	}
> > +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> >  }
> >  
> >  static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> > @@ -6277,22 +6277,23 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum pipe pipe = crtc->pipe;
> >  
> > -	if (crtc_state->pch_pfit.enabled) {
> > -		/* Force use of hard-coded filter coefficients
> > -		 * as some pre-programmed values are broken,
> > -		 * e.g. x201.
> > -		 */
> > -		if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> > -			intel_de_write(dev_priv, PF_CTL(pipe),
> > -				       PF_ENABLE | PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> > -		else
> > -			intel_de_write(dev_priv, PF_CTL(pipe),
> > -				       PF_ENABLE | PF_FILTER_MED_3x3);
> > -		intel_de_write(dev_priv, PF_WIN_POS(pipe),
> > -			       crtc_state->pch_pfit.pos);
> > -		intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> > -			       crtc_state->pch_pfit.size);
> 
> Why dont we use the intel_de_write_fw() everywhere?

Because no one thought it mattered much. It only really becomes
significant (mainly when lockdep is enabled) when a large number
of registers are written.

> 
> Manasi
> 
> > -	}
> > +	if (!crtc_state->pch_pfit.enabled)
> > +		return;
> > +
> > +	/* Force use of hard-coded filter coefficients
> > +	 * as some pre-programmed values are broken,
> > +	 * e.g. x201.
> > +	 */
> > +	if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> > +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> > +			       PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> > +	else
> > +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> > +			       PF_FILTER_MED_3x3);
> > +	intel_de_write(dev_priv, PF_WIN_POS(pipe),
> > +		       crtc_state->pch_pfit.pos);
> > +	intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> > +		       crtc_state->pch_pfit.size);
> >  }
> >  
> >  void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
> > @@ -7107,11 +7108,12 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> >  
> >  	/* To avoid upsetting the power well on haswell only disable the pfit if
> >  	 * it's in use. The hw state code will make sure we get this right. */
> > -	if (old_crtc_state->pch_pfit.enabled) {
> > -		intel_de_write(dev_priv, PF_CTL(pipe), 0);
> > -		intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> > -		intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> > -	}
> > +	if (!old_crtc_state->pch_pfit.enabled)
> > +		return;
> > +
> > +	intel_de_write(dev_priv, PF_CTL(pipe), 0);
> > +	intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> > +	intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> >  }
> >  
> >  static void ilk_crtc_disable(struct intel_atomic_state *state,
> > @@ -7927,39 +7929,35 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
> >  		(crtc->pipe == PIPE_A || IS_I915G(dev_priv));
> >  }
> >  
> > -static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
> > +static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
> >  {
> > -	u32 pixel_rate;
> > -
> > -	pixel_rate = pipe_config->hw.adjusted_mode.crtc_clock;
> > +	u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
> > +	u32 pfit_size = crtc_state->pch_pfit.size;
> > +	u64 pipe_w, pipe_h, pfit_w, pfit_h;
> >  
> >  	/*
> >  	 * We only use IF-ID interlacing. If we ever use
> >  	 * PF-ID we'll need to adjust the pixel_rate here.
> >  	 */
> >  
> > -	if (pipe_config->pch_pfit.enabled) {
> > -		u64 pipe_w, pipe_h, pfit_w, pfit_h;
> > -		u32 pfit_size = pipe_config->pch_pfit.size;
> > -
> > -		pipe_w = pipe_config->pipe_src_w;
> > -		pipe_h = pipe_config->pipe_src_h;
> > +	if (!crtc_state->pch_pfit.enabled)
> > +		return pixel_rate;
> >  
> > -		pfit_w = (pfit_size >> 16) & 0xFFFF;
> > -		pfit_h = pfit_size & 0xFFFF;
> > -		if (pipe_w < pfit_w)
> > -			pipe_w = pfit_w;
> > -		if (pipe_h < pfit_h)
> > -			pipe_h = pfit_h;
> > +	pipe_w = crtc_state->pipe_src_w;
> > +	pipe_h = crtc_state->pipe_src_h;
> >  
> > -		if (WARN_ON(!pfit_w || !pfit_h))
> > -			return pixel_rate;
> > +	pfit_w = (pfit_size >> 16) & 0xFFFF;
> > +	pfit_h = pfit_size & 0xFFFF;
> > +	if (pipe_w < pfit_w)
> > +		pipe_w = pfit_w;
> > +	if (pipe_h < pfit_h)
> > +		pipe_h = pfit_h;
> >  
> > -		pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> > -				     pfit_w * pfit_h);
> > -	}
> > +	if (WARN_ON(!pfit_w || !pfit_h))
> > +		return pixel_rate;
> >  
> > -	return pixel_rate;
> > +	return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> > +		       pfit_w * pfit_h);
> >  }
> >  
> >  static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> > @@ -9153,9 +9151,9 @@ static bool i9xx_has_pfit(struct drm_i915_private *dev_priv)
> >  		IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
> >  }
> >  
> > -static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> > -				 struct intel_crtc_state *pipe_config)
> > +static void i9xx_get_pfit_config(struct intel_crtc_state *crtc_state)
> >  {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	u32 tmp;
> >  
> > @@ -9175,9 +9173,9 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> >  			return;
> >  	}
> >  
> > -	pipe_config->gmch_pfit.control = tmp;
> > -	pipe_config->gmch_pfit.pgm_ratios = intel_de_read(dev_priv,
> > -							  PFIT_PGM_RATIOS);
> > +	crtc_state->gmch_pfit.control = tmp;
> > +	crtc_state->gmch_pfit.pgm_ratios =
> > +		intel_de_read(dev_priv, PFIT_PGM_RATIOS);
> >  }
> >  
> >  static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> > @@ -9427,7 +9425,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  	intel_get_pipe_timings(crtc, pipe_config);
> >  	intel_get_pipe_src_size(crtc, pipe_config);
> >  
> > -	i9xx_get_pfit_config(crtc, pipe_config);
> > +	i9xx_get_pfit_config(pipe_config);
> >  
> >  	if (INTEL_GEN(dev_priv) >= 4) {
> >  		/* No way to read it out on pipes B and C */
> > @@ -10393,37 +10391,37 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
> >  				     &pipe_config->fdi_m_n, NULL);
> >  }
> >  
> > -static void skl_get_pfit_config(struct intel_crtc *crtc,
> > -				struct intel_crtc_state *pipe_config)
> > +static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > -	struct intel_crtc_scaler_state *scaler_state = &pipe_config->scaler_state;
> > -	u32 ps_ctrl = 0;
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
> >  	int id = -1;
> >  	int i;
> >  
> >  	/* find scaler attached to this pipe */
> >  	for (i = 0; i < crtc->num_scalers; i++) {
> > -		ps_ctrl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> > -		if (ps_ctrl & PS_SCALER_EN && !(ps_ctrl & PS_PLANE_SEL_MASK)) {
> > -			id = i;
> > -			pipe_config->pch_pfit.enabled = true;
> > -			pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> > -								  SKL_PS_WIN_POS(crtc->pipe, i));
> > -			pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> > -								   SKL_PS_WIN_SZ(crtc->pipe, i));
> > -			scaler_state->scalers[i].in_use = true;
> > -			break;
> > -		}
> > +		u32 tmp;
> > +
> > +		tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> > +		if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
> > +			continue;
> > +
> > +		id = i;
> > +		crtc_state->pch_pfit.enabled = true;
> > +		crtc_state->pch_pfit.pos =
> > +			intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
> > +		crtc_state->pch_pfit.size =
> > +			intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
> > +		scaler_state->scalers[i].in_use = true;
> > +		break;
> >  	}
> >  
> >  	scaler_state->scaler_id = id;
> > -	if (id >= 0) {
> > +	if (id >= 0)
> >  		scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
> > -	} else {
> > +	else
> >  		scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > -	}
> >  }
> >  
> >  static void
> > @@ -10559,30 +10557,29 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
> >  	kfree(intel_fb);
> >  }
> >  
> > -static void ilk_get_pfit_config(struct intel_crtc *crtc,
> > -				struct intel_crtc_state *pipe_config)
> > +static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	u32 tmp;
> >  
> >  	tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
> > +	if ((tmp & PF_ENABLE) == 0)
> > +		return;
> >  
> > -	if (tmp & PF_ENABLE) {
> > -		pipe_config->pch_pfit.enabled = true;
> > -		pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> > -							  PF_WIN_POS(crtc->pipe));
> > -		pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> > -							   PF_WIN_SZ(crtc->pipe));
> > -
> > -		/* We currently do not free assignements of panel fitters on
> > -		 * ivb/hsw (since we don't use the higher upscaling modes which
> > -		 * differentiates them) so just WARN about this case for now. */
> > -		if (IS_GEN(dev_priv, 7)) {
> > -			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
> > -				PF_PIPE_SEL_IVB(crtc->pipe));
> > -		}
> > -	}
> > +	crtc_state->pch_pfit.enabled = true;
> > +	crtc_state->pch_pfit.pos =
> > +		intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
> > +	crtc_state->pch_pfit.size =
> > +		intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
> > +
> > +	/*
> > +	 * We currently do not free assignements of panel fitters on
> > +	 * ivb/hsw (since we don't use the higher upscaling modes which
> > +	 * differentiates them) so just WARN about this case for now.
> > +	 */
> > +	WARN_ON(IS_GEN(dev_priv, 7) &&
> > +		(tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
> >  }
> >  
> >  static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> > @@ -10694,7 +10691,7 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> >  	intel_get_pipe_timings(crtc, pipe_config);
> >  	intel_get_pipe_src_size(crtc, pipe_config);
> >  
> > -	ilk_get_pfit_config(crtc, pipe_config);
> > +	ilk_get_pfit_config(pipe_config);
> >  
> >  	ret = true;
> >  
> > @@ -11219,9 +11216,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> >  		power_domain_mask |= BIT_ULL(power_domain);
> >  
> >  		if (INTEL_GEN(dev_priv) >= 9)
> > -			skl_get_pfit_config(crtc, pipe_config);
> > +			skl_get_pfit_config(pipe_config);
> >  		else
> > -			ilk_get_pfit_config(crtc, pipe_config);
> > +			ilk_get_pfit_config(pipe_config);
> >  	}
> >  
> >  	if (hsw_crtc_supports_ips(crtc)) {
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/8] drm/i915: Flatten a bunch of the pfit functions

[Manasi Navare]

On Thu, Apr 02, 2020 at 04:55:06PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 01, 2020 at 04:53:23PM -0700, Manasi Navare wrote:
> > On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Most of the pfit functions are of the form:
> > > 
> > > func()
> > > {
> > > 	if (pfit_enabled) {
> > > 		...
> > > 	}
> > > }
> > > 
> > > Flip the pfit_enabled check around to flatten the functions.
> > > 
> > > And while we're touching all this let's do the usual
> > > s/pipe_config/crtc_state/ replacement.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 233 +++++++++----------
> > >  1 file changed, 115 insertions(+), 118 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index becc6322b7dc..796e27c4aece 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6233,42 +6233,42 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
> > >  	enum pipe pipe = crtc->pipe;
> > >  	const struct intel_crtc_scaler_state *scaler_state =
> > >  		&crtc_state->scaler_state;
> > > +	u16 uv_rgb_hphase, uv_rgb_vphase;
> > > +	int pfit_w, pfit_h, hscale, vscale;
> > > +	unsigned long irqflags;
> > > +	int id;
> > >  
> > > -	if (crtc_state->pch_pfit.enabled) {
> > > -		u16 uv_rgb_hphase, uv_rgb_vphase;
> > > -		int pfit_w, pfit_h, hscale, vscale;
> > > -		unsigned long irqflags;
> > > -		int id;
> > > +	if (!crtc_state->pch_pfit.enabled)
> > > +		return;
> > >  
> > > -		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> > > -			return;
> > > +	if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> > > +		return;
> > >  
> > > -		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> > > -		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> > > +	pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> > > +	pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> > >  
> > > -		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > > -		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> > > +	hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > > +	vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> > >  
> > > -		uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> > > -		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> > > +	uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> > > +	uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> > >  
> > > -		id = scaler_state->scaler_id;
> > > +	id = scaler_state->scaler_id;
> > >  
> > > -		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > > +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > >  
> > > -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> > > -				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> > > -		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > > -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> > > -		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> > > -				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> > > -		intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> > > -				  crtc_state->pch_pfit.pos);
> > > -		intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> > > -				  crtc_state->pch_pfit.size);
> > > +	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> > > +			  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> > > +	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > > +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> > > +	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> > > +			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> > > +	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> > > +			  crtc_state->pch_pfit.pos);
> > > +	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> > > +			  crtc_state->pch_pfit.size);
> > >  
> > > -		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > > -	}
> > > +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > >  }
> > >  
> > >  static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> > > @@ -6277,22 +6277,23 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	enum pipe pipe = crtc->pipe;
> > >  
> > > -	if (crtc_state->pch_pfit.enabled) {
> > > -		/* Force use of hard-coded filter coefficients
> > > -		 * as some pre-programmed values are broken,
> > > -		 * e.g. x201.
> > > -		 */
> > > -		if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> > > -			intel_de_write(dev_priv, PF_CTL(pipe),
> > > -				       PF_ENABLE | PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> > > -		else
> > > -			intel_de_write(dev_priv, PF_CTL(pipe),
> > > -				       PF_ENABLE | PF_FILTER_MED_3x3);
> > > -		intel_de_write(dev_priv, PF_WIN_POS(pipe),
> > > -			       crtc_state->pch_pfit.pos);
> > > -		intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> > > -			       crtc_state->pch_pfit.size);
> > 
> > Why dont we use the intel_de_write_fw() everywhere?
> 
> Because no one thought it mattered much. It only really becomes
> significant (mainly when lockdep is enabled) when a large number
> of registers are written.

Okay everything else flattening things in the functions make sense for readability

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

Manasi
> 
> > 
> > Manasi
> > 
> > > -	}
> > > +	if (!crtc_state->pch_pfit.enabled)
> > > +		return;
> > > +
> > > +	/* Force use of hard-coded filter coefficients
> > > +	 * as some pre-programmed values are broken,
> > > +	 * e.g. x201.
> > > +	 */
> > > +	if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> > > +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> > > +			       PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> > > +	else
> > > +		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> > > +			       PF_FILTER_MED_3x3);
> > > +	intel_de_write(dev_priv, PF_WIN_POS(pipe),
> > > +		       crtc_state->pch_pfit.pos);
> > > +	intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> > > +		       crtc_state->pch_pfit.size);
> > >  }
> > >  
> > >  void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
> > > @@ -7107,11 +7108,12 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> > >  
> > >  	/* To avoid upsetting the power well on haswell only disable the pfit if
> > >  	 * it's in use. The hw state code will make sure we get this right. */
> > > -	if (old_crtc_state->pch_pfit.enabled) {
> > > -		intel_de_write(dev_priv, PF_CTL(pipe), 0);
> > > -		intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> > > -		intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> > > -	}
> > > +	if (!old_crtc_state->pch_pfit.enabled)
> > > +		return;
> > > +
> > > +	intel_de_write(dev_priv, PF_CTL(pipe), 0);
> > > +	intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> > > +	intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> > >  }
> > >  
> > >  static void ilk_crtc_disable(struct intel_atomic_state *state,
> > > @@ -7927,39 +7929,35 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
> > >  		(crtc->pipe == PIPE_A || IS_I915G(dev_priv));
> > >  }
> > >  
> > > -static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
> > > +static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
> > >  {
> > > -	u32 pixel_rate;
> > > -
> > > -	pixel_rate = pipe_config->hw.adjusted_mode.crtc_clock;
> > > +	u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
> > > +	u32 pfit_size = crtc_state->pch_pfit.size;
> > > +	u64 pipe_w, pipe_h, pfit_w, pfit_h;
> > >  
> > >  	/*
> > >  	 * We only use IF-ID interlacing. If we ever use
> > >  	 * PF-ID we'll need to adjust the pixel_rate here.
> > >  	 */
> > >  
> > > -	if (pipe_config->pch_pfit.enabled) {
> > > -		u64 pipe_w, pipe_h, pfit_w, pfit_h;
> > > -		u32 pfit_size = pipe_config->pch_pfit.size;
> > > -
> > > -		pipe_w = pipe_config->pipe_src_w;
> > > -		pipe_h = pipe_config->pipe_src_h;
> > > +	if (!crtc_state->pch_pfit.enabled)
> > > +		return pixel_rate;
> > >  
> > > -		pfit_w = (pfit_size >> 16) & 0xFFFF;
> > > -		pfit_h = pfit_size & 0xFFFF;
> > > -		if (pipe_w < pfit_w)
> > > -			pipe_w = pfit_w;
> > > -		if (pipe_h < pfit_h)
> > > -			pipe_h = pfit_h;
> > > +	pipe_w = crtc_state->pipe_src_w;
> > > +	pipe_h = crtc_state->pipe_src_h;
> > >  
> > > -		if (WARN_ON(!pfit_w || !pfit_h))
> > > -			return pixel_rate;
> > > +	pfit_w = (pfit_size >> 16) & 0xFFFF;
> > > +	pfit_h = pfit_size & 0xFFFF;
> > > +	if (pipe_w < pfit_w)
> > > +		pipe_w = pfit_w;
> > > +	if (pipe_h < pfit_h)
> > > +		pipe_h = pfit_h;
> > >  
> > > -		pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> > > -				     pfit_w * pfit_h);
> > > -	}
> > > +	if (WARN_ON(!pfit_w || !pfit_h))
> > > +		return pixel_rate;
> > >  
> > > -	return pixel_rate;
> > > +	return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> > > +		       pfit_w * pfit_h);
> > >  }
> > >  
> > >  static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> > > @@ -9153,9 +9151,9 @@ static bool i9xx_has_pfit(struct drm_i915_private *dev_priv)
> > >  		IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
> > >  }
> > >  
> > > -static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> > > -				 struct intel_crtc_state *pipe_config)
> > > +static void i9xx_get_pfit_config(struct intel_crtc_state *crtc_state)
> > >  {
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	u32 tmp;
> > >  
> > > @@ -9175,9 +9173,9 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> > >  			return;
> > >  	}
> > >  
> > > -	pipe_config->gmch_pfit.control = tmp;
> > > -	pipe_config->gmch_pfit.pgm_ratios = intel_de_read(dev_priv,
> > > -							  PFIT_PGM_RATIOS);
> > > +	crtc_state->gmch_pfit.control = tmp;
> > > +	crtc_state->gmch_pfit.pgm_ratios =
> > > +		intel_de_read(dev_priv, PFIT_PGM_RATIOS);
> > >  }
> > >  
> > >  static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> > > @@ -9427,7 +9425,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > >  	intel_get_pipe_timings(crtc, pipe_config);
> > >  	intel_get_pipe_src_size(crtc, pipe_config);
> > >  
> > > -	i9xx_get_pfit_config(crtc, pipe_config);
> > > +	i9xx_get_pfit_config(pipe_config);
> > >  
> > >  	if (INTEL_GEN(dev_priv) >= 4) {
> > >  		/* No way to read it out on pipes B and C */
> > > @@ -10393,37 +10391,37 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
> > >  				     &pipe_config->fdi_m_n, NULL);
> > >  }
> > >  
> > > -static void skl_get_pfit_config(struct intel_crtc *crtc,
> > > -				struct intel_crtc_state *pipe_config)
> > > +static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
> > >  {
> > > -	struct drm_device *dev = crtc->base.dev;
> > > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > > -	struct intel_crtc_scaler_state *scaler_state = &pipe_config->scaler_state;
> > > -	u32 ps_ctrl = 0;
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > +	struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
> > >  	int id = -1;
> > >  	int i;
> > >  
> > >  	/* find scaler attached to this pipe */
> > >  	for (i = 0; i < crtc->num_scalers; i++) {
> > > -		ps_ctrl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> > > -		if (ps_ctrl & PS_SCALER_EN && !(ps_ctrl & PS_PLANE_SEL_MASK)) {
> > > -			id = i;
> > > -			pipe_config->pch_pfit.enabled = true;
> > > -			pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> > > -								  SKL_PS_WIN_POS(crtc->pipe, i));
> > > -			pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> > > -								   SKL_PS_WIN_SZ(crtc->pipe, i));
> > > -			scaler_state->scalers[i].in_use = true;
> > > -			break;
> > > -		}
> > > +		u32 tmp;
> > > +
> > > +		tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> > > +		if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
> > > +			continue;
> > > +
> > > +		id = i;
> > > +		crtc_state->pch_pfit.enabled = true;
> > > +		crtc_state->pch_pfit.pos =
> > > +			intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
> > > +		crtc_state->pch_pfit.size =
> > > +			intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
> > > +		scaler_state->scalers[i].in_use = true;
> > > +		break;
> > >  	}
> > >  
> > >  	scaler_state->scaler_id = id;
> > > -	if (id >= 0) {
> > > +	if (id >= 0)
> > >  		scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
> > > -	} else {
> > > +	else
> > >  		scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > > -	}
> > >  }
> > >  
> > >  static void
> > > @@ -10559,30 +10557,29 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
> > >  	kfree(intel_fb);
> > >  }
> > >  
> > > -static void ilk_get_pfit_config(struct intel_crtc *crtc,
> > > -				struct intel_crtc_state *pipe_config)
> > > +static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
> > >  {
> > > -	struct drm_device *dev = crtc->base.dev;
> > > -	struct drm_i915_private *dev_priv = to_i915(dev);
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	u32 tmp;
> > >  
> > >  	tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
> > > +	if ((tmp & PF_ENABLE) == 0)
> > > +		return;
> > >  
> > > -	if (tmp & PF_ENABLE) {
> > > -		pipe_config->pch_pfit.enabled = true;
> > > -		pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> > > -							  PF_WIN_POS(crtc->pipe));
> > > -		pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> > > -							   PF_WIN_SZ(crtc->pipe));
> > > -
> > > -		/* We currently do not free assignements of panel fitters on
> > > -		 * ivb/hsw (since we don't use the higher upscaling modes which
> > > -		 * differentiates them) so just WARN about this case for now. */
> > > -		if (IS_GEN(dev_priv, 7)) {
> > > -			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
> > > -				PF_PIPE_SEL_IVB(crtc->pipe));
> > > -		}
> > > -	}
> > > +	crtc_state->pch_pfit.enabled = true;
> > > +	crtc_state->pch_pfit.pos =
> > > +		intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
> > > +	crtc_state->pch_pfit.size =
> > > +		intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
> > > +
> > > +	/*
> > > +	 * We currently do not free assignements of panel fitters on
> > > +	 * ivb/hsw (since we don't use the higher upscaling modes which
> > > +	 * differentiates them) so just WARN about this case for now.
> > > +	 */
> > > +	WARN_ON(IS_GEN(dev_priv, 7) &&
> > > +		(tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
> > >  }
> > >  
> > >  static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> > > @@ -10694,7 +10691,7 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> > >  	intel_get_pipe_timings(crtc, pipe_config);
> > >  	intel_get_pipe_src_size(crtc, pipe_config);
> > >  
> > > -	ilk_get_pfit_config(crtc, pipe_config);
> > > +	ilk_get_pfit_config(pipe_config);
> > >  
> > >  	ret = true;
> > >  
> > > @@ -11219,9 +11216,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> > >  		power_domain_mask |= BIT_ULL(power_domain);
> > >  
> > >  		if (INTEL_GEN(dev_priv) >= 9)
> > > -			skl_get_pfit_config(crtc, pipe_config);
> > > +			skl_get_pfit_config(pipe_config);
> > >  		else
> > > -			ilk_get_pfit_config(crtc, pipe_config);
> > > +			ilk_get_pfit_config(pipe_config);
> > >  	}
> > >  
> > >  	if (hsw_crtc_supports_ips(crtc)) {
> > > -- 
> > > 2.24.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 5/8] drm/i915: Use drm_rect to store the pfit window pos/size

[Ville Syrjala]

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

Make things a bit more abstract by replacing the pch_pfit.pos/size
raw register values with a drm_rect. Makes it slighly more convenient
to eg. compute the scaling factors.

v2: Use drm_rect_init()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 101 +++++++++++-------
 .../drm/i915/display/intel_display_types.h    |   3 +-
 drivers/gpu/drm/i915/display/intel_panel.c    |  13 ++-
 3 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 796e27c4aece..19f3fef11b0d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6108,10 +6108,8 @@ static int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state)
 	int width, height;
 
 	if (crtc_state->pch_pfit.enabled) {
-		u32 pfit_size = crtc_state->pch_pfit.size;
-
-		width = pfit_size >> 16;
-		height = pfit_size & 0xffff;
+		width = drm_rect_width(&crtc_state->pch_pfit.dst);
+		height = drm_rect_height(&crtc_state->pch_pfit.dst);
 	} else {
 		width = adjusted_mode->crtc_hdisplay;
 		height = adjusted_mode->crtc_vdisplay;
@@ -6230,11 +6228,20 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
 	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;
-	int pfit_w, pfit_h, hscale, vscale;
+	enum pipe pipe = crtc->pipe;
+	int width = drm_rect_width(dst);
+	int height = drm_rect_height(dst);
+	int x = dst->x1;
+	int y = dst->y1;
+	int hscale, vscale;
 	unsigned long irqflags;
 	int id;
 
@@ -6244,11 +6251,8 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
 		return;
 
-	pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
-	pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
-
-	hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
-	vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
+	hscale = drm_rect_calc_hscale(&src, dst, 0, INT_MAX);
+	vscale = drm_rect_calc_vscale(&src, dst, 0, INT_MAX);
 
 	uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
 	uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
@@ -6264,9 +6268,9 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
 			  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
 	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
-			  crtc_state->pch_pfit.pos);
+			  x << 16 | y);
 	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
-			  crtc_state->pch_pfit.size);
+			  width << 16 | height);
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
@@ -6275,7 +6279,12 @@ static void ilk_pfit_enable(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);
+	const struct drm_rect *dst = &crtc_state->pch_pfit.dst;
 	enum pipe pipe = crtc->pipe;
+	int width = drm_rect_width(dst);
+	int height = drm_rect_height(dst);
+	int x = dst->x1;
+	int y = dst->y1;
 
 	if (!crtc_state->pch_pfit.enabled)
 		return;
@@ -6290,10 +6299,8 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
 	else
 		intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
 			       PF_FILTER_MED_3x3);
-	intel_de_write(dev_priv, PF_WIN_POS(pipe),
-		       crtc_state->pch_pfit.pos);
-	intel_de_write(dev_priv, PF_WIN_SZ(pipe),
-		       crtc_state->pch_pfit.size);
+	intel_de_write(dev_priv, PF_WIN_POS(pipe), x << 16 | y);
+	intel_de_write(dev_priv, PF_WIN_SZ(pipe), width << 16 | height);
 }
 
 void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
@@ -7932,8 +7939,7 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
 static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
 {
 	u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
-	u32 pfit_size = crtc_state->pch_pfit.size;
-	u64 pipe_w, pipe_h, pfit_w, pfit_h;
+	unsigned int pipe_w, pipe_h, pfit_w, pfit_h;
 
 	/*
 	 * We only use IF-ID interlacing. If we ever use
@@ -7946,8 +7952,9 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
 	pipe_w = crtc_state->pipe_src_w;
 	pipe_h = crtc_state->pipe_src_h;
 
-	pfit_w = (pfit_size >> 16) & 0xFFFF;
-	pfit_h = pfit_size & 0xFFFF;
+	pfit_w = drm_rect_width(&crtc_state->pch_pfit.dst);
+	pfit_h = drm_rect_height(&crtc_state->pch_pfit.dst);
+
 	if (pipe_w < pfit_w)
 		pipe_w = pfit_w;
 	if (pipe_h < pfit_h)
@@ -10391,6 +10398,14 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
 				     &pipe_config->fdi_m_n, NULL);
 }
 
+static void ilk_get_pfit_pos_size(struct intel_crtc_state *crtc_state,
+				  u32 pos, u32 size)
+{
+	drm_rect_init(&crtc_state->pch_pfit.dst,
+		      pos >> 16, pos & 0xffff,
+		      size >> 16, size & 0xffff);
+}
+
 static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -10401,18 +10416,20 @@ static void skl_get_pfit_config(struct intel_crtc_state *crtc_state)
 
 	/* find scaler attached to this pipe */
 	for (i = 0; i < crtc->num_scalers; i++) {
-		u32 tmp;
+		u32 ctl, pos, size;
 
-		tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
-		if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
+		ctl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
+		if ((ctl & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
 			continue;
 
 		id = i;
 		crtc_state->pch_pfit.enabled = true;
-		crtc_state->pch_pfit.pos =
-			intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
-		crtc_state->pch_pfit.size =
-			intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
+
+		pos = intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
+		size = intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
+
+		ilk_get_pfit_pos_size(crtc_state, pos, size);
+
 		scaler_state->scalers[i].in_use = true;
 		break;
 	}
@@ -10561,17 +10578,18 @@ static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 tmp;
+	u32 ctl, pos, size;
 
-	tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
-	if ((tmp & PF_ENABLE) == 0)
+	ctl = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
+	if ((ctl & PF_ENABLE) == 0)
 		return;
 
 	crtc_state->pch_pfit.enabled = true;
-	crtc_state->pch_pfit.pos =
-		intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
-	crtc_state->pch_pfit.size =
-		intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
+
+	pos = intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
+	size = intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
+
+	ilk_get_pfit_pos_size(crtc_state, pos, size);
 
 	/*
 	 * We currently do not free assignements of panel fitters on
@@ -10579,7 +10597,7 @@ static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
 	 * differentiates them) so just WARN about this case for now.
 	 */
 	WARN_ON(IS_GEN(dev_priv, 7) &&
-		(tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
+		(ctl & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
 }
 
 static bool ilk_get_pipe_config(struct intel_crtc *crtc,
@@ -13196,9 +13214,8 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 			    pipe_config->gmch_pfit.lvds_border_bits);
 	else
 		drm_dbg_kms(&dev_priv->drm,
-			    "pch pfit: pos: 0x%08x, size: 0x%08x, %s, force thru: %s\n",
-			    pipe_config->pch_pfit.pos,
-			    pipe_config->pch_pfit.size,
+			    "pch pfit: " DRM_RECT_FMT ", %s, force thru: %s\n",
+			    DRM_RECT_ARG(&pipe_config->pch_pfit.dst),
 			    enableddisabled(pipe_config->pch_pfit.enabled),
 			    yesno(pipe_config->pch_pfit.force_thru));
 
@@ -13949,8 +13966,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 		PIPE_CONF_CHECK_BOOL(pch_pfit.enabled);
 		if (current_config->pch_pfit.enabled) {
-			PIPE_CONF_CHECK_X(pch_pfit.pos);
-			PIPE_CONF_CHECK_X(pch_pfit.size);
+			PIPE_CONF_CHECK_I(pch_pfit.dst.x1);
+			PIPE_CONF_CHECK_I(pch_pfit.dst.y1);
+			PIPE_CONF_CHECK_I(pch_pfit.dst.x2);
+			PIPE_CONF_CHECK_I(pch_pfit.dst.y2);
 		}
 
 		PIPE_CONF_CHECK_I(scaler_state.scaler_id);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 283c622f8ba1..38eb1036f32b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -937,8 +937,7 @@ struct intel_crtc_state {
 
 	/* Panel fitter placement and size for Ironlake+ */
 	struct {
-		u32 pos;
-		u32 size;
+		struct drm_rect dst;
 		bool enabled;
 		bool force_thru;
 	} pch_pfit;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 8b0730f4c442..972fff7e2ba0 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -182,13 +182,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 			int fitting_mode)
 {
 	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
-	int x = 0, y = 0, width = 0, height = 0;
+	int x, y, width, height;
 
 	/* Native modes don't need fitting */
 	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
 	    pipe_config->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
-		goto done;
+		return;
 
 	switch (fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
@@ -234,14 +234,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 		break;
 
 	default:
-		WARN(1, "bad panel fit mode: %d\n", fitting_mode);
+		MISSING_CASE(fitting_mode);
 		return;
 	}
 
-done:
-	pipe_config->pch_pfit.pos = (x << 16) | y;
-	pipe_config->pch_pfit.size = (width << 16) | height;
-	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+	drm_rect_init(&pipe_config->pch_pfit.dst,
+		      x, y, width, height);
+	pipe_config->pch_pfit.enabled = true;
 }
 
 static void
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 6/8] drm/i915: s/pipe_config/crtc_state/ in pfit functions

[Ville Syrjala]

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

Follow the new naming convention and call the crtc state
"crtc_state", and while at it drop the redundant crtc argument.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c     |  3 +-
 drivers/gpu/drm/i915/display/intel_dp.c    |  8 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c  |  5 +-
 drivers/gpu/drm/i915/display/intel_lvds.c  |  4 +-
 drivers/gpu/drm/i915/display/intel_panel.c | 93 +++++++++++-----------
 drivers/gpu/drm/i915/display/intel_panel.h |  6 +-
 drivers/gpu/drm/i915/display/vlv_dsi.c     |  5 +-
 7 files changed, 58 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index d842e280699d..7481ec04883a 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1420,7 +1420,6 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
 						   base);
 	struct intel_connector *intel_connector = intel_dsi->attached_connector;
-	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	const struct drm_display_mode *fixed_mode =
 					intel_connector->panel.fixed_mode;
 	struct drm_display_mode *adjusted_mode =
@@ -1428,7 +1427,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
-	intel_pch_panel_fitting(crtc, pipe_config, conn_state->scaling_mode);
+	intel_pch_panel_fitting(pipe_config, conn_state->scaling_mode);
 
 	adjusted_mode->flags = 0;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a827eac8acc2..a352fdcba20c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2306,7 +2306,6 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 	const struct drm_display_info *info = &connector->display_info;
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
 	if (!drm_mode_is_420_only(info, adjusted_mode) ||
 	    !intel_dp_get_colorimetry_status(intel_dp) ||
@@ -2315,7 +2314,7 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(crtc, crtc_state, DRM_MODE_SCALE_FULLSCREEN);
+	intel_pch_panel_fitting(crtc_state, DRM_MODE_SCALE_FULLSCREEN);
 
 	return 0;
 }
@@ -2374,7 +2373,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
 	enum port port = encoder->port;
-	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	struct intel_digital_connector_state *intel_conn_state =
 		to_intel_digital_connector_state(conn_state);
@@ -2408,10 +2406,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 				       adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(intel_crtc, pipe_config,
+			intel_gmch_panel_fitting(pipe_config,
 						 conn_state->scaling_mode);
 		else
-			intel_pch_panel_fitting(intel_crtc, pipe_config,
+			intel_pch_panel_fitting(pipe_config,
 						conn_state->scaling_mode);
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 1e42b01045b1..22345880c8f5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2299,8 +2299,6 @@ static bool
 intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 			   struct intel_crtc_state *config)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(config->uapi.crtc);
-
 	if (!connector->ycbcr_420_allowed) {
 		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
 		return false;
@@ -2308,8 +2306,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
 
 	config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(intel_crtc, config,
-				DRM_MODE_SCALE_FULLSCREEN);
+	intel_pch_panel_fitting(config, DRM_MODE_SCALE_FULLSCREEN);
 
 	return true;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index b7ad0b534790..bf8f4d79d083 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -430,10 +430,10 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	if (HAS_PCH_SPLIT(dev_priv)) {
 		pipe_config->has_pch_encoder = true;
 
-		intel_pch_panel_fitting(intel_crtc, pipe_config,
+		intel_pch_panel_fitting(pipe_config,
 					conn_state->scaling_mode);
 	} else {
-		intel_gmch_panel_fitting(intel_crtc, pipe_config,
+		intel_gmch_panel_fitting(pipe_config,
 					 conn_state->scaling_mode);
 
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 972fff7e2ba0..a36f17ef97f1 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -177,23 +177,23 @@ intel_panel_vbt_fixed_mode(struct intel_connector *connector)
 
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
-intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
-			struct intel_crtc_state *pipe_config,
+intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 			int fitting_mode)
 {
-	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
 	int x, y, width, height;
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
-	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
-	    pipe_config->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
+	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h &&
+	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		return;
 
 	switch (fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
-		width = pipe_config->pipe_src_w;
-		height = pipe_config->pipe_src_h;
+		width = crtc_state->pipe_src_w;
+		height = crtc_state->pipe_src_h;
 		x = (adjusted_mode->crtc_hdisplay - width + 1)/2;
 		y = (adjusted_mode->crtc_vdisplay - height + 1)/2;
 		break;
@@ -202,18 +202,18 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 		/* Scale but preserve the aspect ratio */
 		{
 			u32 scaled_width = adjusted_mode->crtc_hdisplay
-				* pipe_config->pipe_src_h;
-			u32 scaled_height = pipe_config->pipe_src_w
+				* crtc_state->pipe_src_h;
+			u32 scaled_height = crtc_state->pipe_src_w
 				* adjusted_mode->crtc_vdisplay;
 			if (scaled_width > scaled_height) { /* pillar */
-				width = scaled_height / pipe_config->pipe_src_h;
+				width = scaled_height / crtc_state->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 / pipe_config->pipe_src_w;
+				height = scaled_width / crtc_state->pipe_src_w;
 				if (height & 1)
 				    height++;
 				y = (adjusted_mode->crtc_vdisplay - height + 1) / 2;
@@ -238,9 +238,9 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 		return;
 	}
 
-	drm_rect_init(&pipe_config->pch_pfit.dst,
+	drm_rect_init(&crtc_state->pch_pfit.dst,
 		      x, y, width, height);
-	pipe_config->pch_pfit.enabled = true;
+	crtc_state->pch_pfit.enabled = true;
 }
 
 static void
@@ -299,13 +299,14 @@ static inline u32 panel_fitter_scaling(u32 source, u32 target)
 	return (FACTOR * ratio + FACTOR/2) / FACTOR;
 }
 
-static void i965_scale_aspect(struct intel_crtc_state *pipe_config,
+static void i965_scale_aspect(struct intel_crtc_state *crtc_state,
 			      u32 *pfit_control)
 {
-	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
 	u32 scaled_width = adjusted_mode->crtc_hdisplay *
-		pipe_config->pipe_src_h;
-	u32 scaled_height = pipe_config->pipe_src_w *
+		crtc_state->pipe_src_h;
+	u32 scaled_height = crtc_state->pipe_src_w *
 		adjusted_mode->crtc_vdisplay;
 
 	/* 965+ is easy, it does everything in hw */
@@ -315,18 +316,18 @@ static void i965_scale_aspect(struct intel_crtc_state *pipe_config,
 	else if (scaled_width < scaled_height)
 		*pfit_control |= PFIT_ENABLE |
 			PFIT_SCALING_LETTER;
-	else if (adjusted_mode->crtc_hdisplay != pipe_config->pipe_src_w)
+	else if (adjusted_mode->crtc_hdisplay != crtc_state->pipe_src_w)
 		*pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
 }
 
-static void i9xx_scale_aspect(struct intel_crtc_state *pipe_config,
+static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 			      u32 *pfit_control, u32 *pfit_pgm_ratios,
 			      u32 *border)
 {
-	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	u32 scaled_width = adjusted_mode->crtc_hdisplay *
-		pipe_config->pipe_src_h;
-	u32 scaled_height = pipe_config->pipe_src_w *
+		crtc_state->pipe_src_h;
+	u32 scaled_height = crtc_state->pipe_src_w *
 		adjusted_mode->crtc_vdisplay;
 	u32 bits;
 
@@ -338,11 +339,11 @@ static void i9xx_scale_aspect(struct intel_crtc_state *pipe_config,
 	if (scaled_width > scaled_height) { /* pillar */
 		centre_horizontally(adjusted_mode,
 				    scaled_height /
-				    pipe_config->pipe_src_h);
+				    crtc_state->pipe_src_h);
 
 		*border = LVDS_BORDER_ENABLE;
-		if (pipe_config->pipe_src_h != adjusted_mode->crtc_vdisplay) {
-			bits = panel_fitter_scaling(pipe_config->pipe_src_h,
+		if (crtc_state->pipe_src_h != adjusted_mode->crtc_vdisplay) {
+			bits = panel_fitter_scaling(crtc_state->pipe_src_h,
 						    adjusted_mode->crtc_vdisplay);
 
 			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
@@ -354,11 +355,11 @@ static void i9xx_scale_aspect(struct intel_crtc_state *pipe_config,
 	} else if (scaled_width < scaled_height) { /* letter */
 		centre_vertically(adjusted_mode,
 				  scaled_width /
-				  pipe_config->pipe_src_w);
+				  crtc_state->pipe_src_w);
 
 		*border = LVDS_BORDER_ENABLE;
-		if (pipe_config->pipe_src_w != adjusted_mode->crtc_hdisplay) {
-			bits = panel_fitter_scaling(pipe_config->pipe_src_w,
+		if (crtc_state->pipe_src_w != adjusted_mode->crtc_hdisplay) {
+			bits = panel_fitter_scaling(crtc_state->pipe_src_w,
 						    adjusted_mode->crtc_hdisplay);
 
 			*pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
@@ -376,17 +377,17 @@ static void i9xx_scale_aspect(struct intel_crtc_state *pipe_config,
 	}
 }
 
-void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
-			      struct intel_crtc_state *pipe_config,
+void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 			      int fitting_mode)
 {
-	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	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 = &pipe_config->hw.adjusted_mode;
+	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
-	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h)
+	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h)
 		goto out;
 
 	switch (fitting_mode) {
@@ -395,16 +396,16 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		 * For centered modes, we have to calculate border widths &
 		 * heights and modify the values programmed into the CRTC.
 		 */
-		centre_horizontally(adjusted_mode, pipe_config->pipe_src_w);
-		centre_vertically(adjusted_mode, pipe_config->pipe_src_h);
+		centre_horizontally(adjusted_mode, crtc_state->pipe_src_w);
+		centre_vertically(adjusted_mode, crtc_state->pipe_src_h);
 		border = LVDS_BORDER_ENABLE;
 		break;
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
 		if (INTEL_GEN(dev_priv) >= 4)
-			i965_scale_aspect(pipe_config, &pfit_control);
+			i965_scale_aspect(crtc_state, &pfit_control);
 		else
-			i9xx_scale_aspect(pipe_config, &pfit_control,
+			i9xx_scale_aspect(crtc_state, &pfit_control,
 					  &pfit_pgm_ratios, &border);
 		break;
 	case DRM_MODE_SCALE_FULLSCREEN:
@@ -412,8 +413,8 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		 * Full scaling, even if it changes the aspect ratio.
 		 * Fortunately this is all done for us in hw.
 		 */
-		if (pipe_config->pipe_src_h != adjusted_mode->crtc_vdisplay ||
-		    pipe_config->pipe_src_w != adjusted_mode->crtc_hdisplay) {
+		if (crtc_state->pipe_src_h != adjusted_mode->crtc_vdisplay ||
+		    crtc_state->pipe_src_w != adjusted_mode->crtc_hdisplay) {
 			pfit_control |= PFIT_ENABLE;
 			if (INTEL_GEN(dev_priv) >= 4)
 				pfit_control |= PFIT_SCALING_AUTO;
@@ -433,7 +434,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 	/* 965+ wants fuzzy fitting */
 	/* FIXME: handle multiple panels by failing gracefully */
 	if (INTEL_GEN(dev_priv) >= 4)
-		pfit_control |= PFIT_PIPE(intel_crtc->pipe) | PFIT_FILTER_FUZZY;
+		pfit_control |= PFIT_PIPE(crtc->pipe) | PFIT_FILTER_FUZZY;
 
 out:
 	if ((pfit_control & PFIT_ENABLE) == 0) {
@@ -442,12 +443,12 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 	}
 
 	/* Make sure pre-965 set dither correctly for 18bpp panels. */
-	if (INTEL_GEN(dev_priv) < 4 && pipe_config->pipe_bpp == 18)
+	if (INTEL_GEN(dev_priv) < 4 && crtc_state->pipe_bpp == 18)
 		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
 
-	pipe_config->gmch_pfit.control = pfit_control;
-	pipe_config->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
-	pipe_config->gmch_pfit.lvds_border_bits = border;
+	crtc_state->gmch_pfit.control = pfit_control;
+	crtc_state->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
+	crtc_state->gmch_pfit.lvds_border_bits = border;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index cedeea443336..e1804e6e8325 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -25,11 +25,9 @@ int intel_panel_init(struct intel_panel *panel,
 void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
-void intel_pch_panel_fitting(struct intel_crtc *crtc,
-			     struct intel_crtc_state *pipe_config,
+void intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 			     int fitting_mode);
-void intel_gmch_panel_fitting(struct intel_crtc *crtc,
-			      struct intel_crtc_state *pipe_config,
+void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 			      int fitting_mode);
 void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
 				    u32 level, u32 max);
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index d07cfad8ce6f..cd2f9f46cf9f 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -267,7 +267,6 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
 	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
 						   base);
 	struct intel_connector *intel_connector = intel_dsi->attached_connector;
-	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	const struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
 	int ret;
@@ -279,10 +278,10 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(crtc, pipe_config,
+			intel_gmch_panel_fitting(pipe_config,
 						 conn_state->scaling_mode);
 		else
-			intel_pch_panel_fitting(crtc, pipe_config,
+			intel_pch_panel_fitting(pipe_config,
 						conn_state->scaling_mode);
 	}
 
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 7/8] drm/i915: Pass connector state to pfit calculations

[Ville Syrjala]

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

Pass the entire connector state to intel_{gmch,pch}_panel_fitting().
For now we just need to get at .scaling_mode but in the future we'll
want access to the margin properties as well.

v2: Deal with intel_dp_ycbcr420_config()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c     |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c    | 17 ++++++++---------
 drivers/gpu/drm/i915/display/intel_hdmi.c  | 12 +++++++-----
 drivers/gpu/drm/i915/display/intel_lvds.c  |  7 ++-----
 drivers/gpu/drm/i915/display/intel_panel.c | 17 ++++++++++-------
 drivers/gpu/drm/i915/display/intel_panel.h |  4 ++--
 drivers/gpu/drm/i915/display/vlv_dsi.c     |  6 ++----
 7 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 7481ec04883a..d5178be48226 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1427,7 +1427,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
-	intel_pch_panel_fitting(pipe_config, conn_state->scaling_mode);
+	intel_pch_panel_fitting(pipe_config, conn_state);
 
 	adjusted_mode->flags = 0;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a352fdcba20c..03845bd7d927 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2300,9 +2300,10 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 
 static int
 intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
-			 struct drm_connector *connector,
-			 struct intel_crtc_state *crtc_state)
+			 struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
 {
+	struct drm_connector *connector = conn_state->connector;
 	const struct drm_display_info *info = &connector->display_info;
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
@@ -2314,7 +2315,7 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(crtc_state, DRM_MODE_SCALE_FULLSCREEN);
+	intel_pch_panel_fitting(crtc_state, conn_state);
 
 	return 0;
 }
@@ -2388,8 +2389,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (lspcon->active)
 		lspcon_ycbcr420_config(&intel_connector->base, pipe_config);
 	else
-		ret = intel_dp_ycbcr420_config(intel_dp, &intel_connector->base,
-					       pipe_config);
+		ret = intel_dp_ycbcr420_config(intel_dp, pipe_config,
+					       conn_state);
 	if (ret)
 		return ret;
 
@@ -2406,11 +2407,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 				       adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(pipe_config,
-						 conn_state->scaling_mode);
+			intel_gmch_panel_fitting(pipe_config, conn_state);
 		else
-			intel_pch_panel_fitting(pipe_config,
-						conn_state->scaling_mode);
+			intel_pch_panel_fitting(pipe_config, conn_state);
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 22345880c8f5..5e78d993ce77 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2296,17 +2296,19 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 }
 
 static bool
-intel_hdmi_ycbcr420_config(struct drm_connector *connector,
-			   struct intel_crtc_state *config)
+intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
 {
+	struct drm_connector *connector = conn_state->connector;
+
 	if (!connector->ycbcr_420_allowed) {
 		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
 		return false;
 	}
 
-	config->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(config, DRM_MODE_SCALE_FULLSCREEN);
+	intel_pch_panel_fitting(crtc_state, conn_state);
 
 	return true;
 }
@@ -2434,7 +2436,7 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 		pipe_config->pixel_multiplier = 2;
 
 	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
-		if (!intel_hdmi_ycbcr420_config(connector, pipe_config)) {
+		if (!intel_hdmi_ycbcr420_config(pipe_config, conn_state)) {
 			DRM_ERROR("Can't support YCBCR420 output\n");
 			return -EINVAL;
 		}
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index bf8f4d79d083..eee6bb6ca53b 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -430,12 +430,9 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	if (HAS_PCH_SPLIT(dev_priv)) {
 		pipe_config->has_pch_encoder = true;
 
-		intel_pch_panel_fitting(pipe_config,
-					conn_state->scaling_mode);
+		intel_pch_panel_fitting(pipe_config, conn_state);
 	} else {
-		intel_gmch_panel_fitting(pipe_config,
-					 conn_state->scaling_mode);
-
+		intel_gmch_panel_fitting(pipe_config, conn_state);
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index a36f17ef97f1..295c07cec19b 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -178,7 +178,7 @@ intel_panel_vbt_fixed_mode(struct intel_connector *connector)
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
-			int fitting_mode)
+			const struct drm_connector_state *conn_state)
 {
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
@@ -190,7 +190,7 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
 		return;
 
-	switch (fitting_mode) {
+	switch (conn_state->scaling_mode) {
 	case DRM_MODE_SCALE_CENTER:
 		width = crtc_state->pipe_src_w;
 		height = crtc_state->pipe_src_h;
@@ -227,6 +227,10 @@ intel_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);
+		/* fall through */
 	case DRM_MODE_SCALE_FULLSCREEN:
 		x = y = 0;
 		width = adjusted_mode->crtc_hdisplay;
@@ -234,7 +238,7 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 		break;
 
 	default:
-		MISSING_CASE(fitting_mode);
+		MISSING_CASE(conn_state->scaling_mode);
 		return;
 	}
 
@@ -378,7 +382,7 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 }
 
 void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
-			      int fitting_mode)
+			      const struct drm_connector_state *conn_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -390,7 +394,7 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h)
 		goto out;
 
-	switch (fitting_mode) {
+	switch (conn_state->scaling_mode) {
 	case DRM_MODE_SCALE_CENTER:
 		/*
 		 * For centered modes, we have to calculate border widths &
@@ -426,8 +430,7 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 		}
 		break;
 	default:
-		drm_WARN(&dev_priv->drm, 1, "bad panel fit mode: %d\n",
-			 fitting_mode);
+		MISSING_CASE(conn_state->scaling_mode);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index e1804e6e8325..e2fa1543a61f 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -26,9 +26,9 @@ void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
 void intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
-			     int fitting_mode);
+			     const struct drm_connector_state *conn_state);
 void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
-			      int fitting_mode);
+			      const struct drm_connector_state *conn_state);
 void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
 				    u32 level, u32 max);
 int intel_panel_setup_backlight(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index cd2f9f46cf9f..97f72d320d87 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -278,11 +278,9 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(pipe_config,
-						 conn_state->scaling_mode);
+			intel_gmch_panel_fitting(pipe_config, conn_state);
 		else
-			intel_pch_panel_fitting(pipe_config,
-						conn_state->scaling_mode);
+			intel_pch_panel_fitting(pipe_config, conn_state);
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 8/8] drm/i915: Have pfit calculations return an error code

[Ville Syrjala]

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

Change intel_{gmch,pch}_panel_fitting() to return a normal
error vs. success int. We'll need this later to validate that
the margin properties aren't misconfigured.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c     | 10 +++++++---
 drivers/gpu/drm/i915/display/intel_dp.c    | 10 +++++-----
 drivers/gpu/drm/i915/display/intel_hdmi.c  | 22 +++++++++++-----------
 drivers/gpu/drm/i915/display/intel_lvds.c  | 13 ++++++++-----
 drivers/gpu/drm/i915/display/intel_panel.c | 19 +++++++++++--------
 drivers/gpu/drm/i915/display/intel_panel.h |  6 +++---
 drivers/gpu/drm/i915/display/vlv_dsi.c     |  6 ++++--
 7 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index d5178be48226..3134b0436040 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1421,13 +1421,17 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 						   base);
 	struct intel_connector *intel_connector = intel_dsi->attached_connector;
 	const struct drm_display_mode *fixed_mode =
-					intel_connector->panel.fixed_mode;
+		intel_connector->panel.fixed_mode;
 	struct drm_display_mode *adjusted_mode =
-					&pipe_config->hw.adjusted_mode;
+		&pipe_config->hw.adjusted_mode;
+	int ret;
 
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
-	intel_pch_panel_fitting(pipe_config, conn_state);
+
+	ret = intel_pch_panel_fitting(pipe_config, conn_state);
+	if (ret)
+		return ret;
 
 	adjusted_mode->flags = 0;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 03845bd7d927..e8ebacb066da 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2315,9 +2315,7 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
 
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(crtc_state, conn_state);
-
-	return 0;
+	return intel_pch_panel_fitting(crtc_state, conn_state);
 }
 
 bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state,
@@ -2407,9 +2405,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 				       adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(pipe_config, conn_state);
+			ret = intel_gmch_panel_fitting(pipe_config, conn_state);
 		else
-			intel_pch_panel_fitting(pipe_config, conn_state);
+			ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 5e78d993ce77..559caee0bb25 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2295,22 +2295,25 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 	return true;
 }
 
-static bool
+static int
 intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
 			   const struct drm_connector_state *conn_state)
 {
 	struct drm_connector *connector = conn_state->connector;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
+
+	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
+		return 0;
 
 	if (!connector->ycbcr_420_allowed) {
 		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
-		return false;
+		return -EINVAL;
 	}
 
 	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 
-	intel_pch_panel_fitting(crtc_state, conn_state);
-
-	return true;
+	return intel_pch_panel_fitting(crtc_state, conn_state);
 }
 
 static int intel_hdmi_port_clock(int clock, int bpc)
@@ -2435,12 +2438,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		pipe_config->pixel_multiplier = 2;
 
-	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
-		if (!intel_hdmi_ycbcr420_config(pipe_config, conn_state)) {
-			DRM_ERROR("Can't support YCBCR420 output\n");
-			return -EINVAL;
-		}
-	}
+	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
+	if (ret)
+		return ret;
 
 	pipe_config->limited_color_range =
 		intel_hdmi_limited_color_range(pipe_config, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index eee6bb6ca53b..fa9e819fd258 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -395,6 +395,7 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	unsigned int lvds_bpp;
+	int ret;
 
 	/* Should never happen!! */
 	if (INTEL_GEN(dev_priv) < 4 && intel_crtc->pipe == 0) {
@@ -427,13 +428,15 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
-	if (HAS_PCH_SPLIT(dev_priv)) {
+	if (HAS_PCH_SPLIT(dev_priv))
 		pipe_config->has_pch_encoder = true;
 
-		intel_pch_panel_fitting(pipe_config, conn_state);
-	} else {
-		intel_gmch_panel_fitting(pipe_config, conn_state);
-	}
+	if (HAS_GMCH(dev_priv))
+		ret = intel_gmch_panel_fitting(pipe_config, conn_state);
+	else
+		ret = intel_pch_panel_fitting(pipe_config, conn_state);
+	if (ret)
+		return ret;
 
 	/*
 	 * XXX: It would be nice to support lower refresh rates on the
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 295c07cec19b..96e880fcf666 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -176,9 +176,8 @@ intel_panel_vbt_fixed_mode(struct intel_connector *connector)
 }
 
 /* adjusted_mode has been preset to be the panel's fixed mode */
-void
-intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
-			const struct drm_connector_state *conn_state)
+int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
+			    const struct drm_connector_state *conn_state)
 {
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
@@ -188,7 +187,7 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
 	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h &&
 	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
-		return;
+		return 0;
 
 	switch (conn_state->scaling_mode) {
 	case DRM_MODE_SCALE_CENTER:
@@ -239,12 +238,14 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
 
 	default:
 		MISSING_CASE(conn_state->scaling_mode);
-		return;
+		return -EINVAL;
 	}
 
 	drm_rect_init(&crtc_state->pch_pfit.dst,
 		      x, y, width, height);
 	crtc_state->pch_pfit.enabled = true;
+
+	return 0;
 }
 
 static void
@@ -381,8 +382,8 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
 	}
 }
 
-void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
-			      const struct drm_connector_state *conn_state)
+int intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
+			     const struct drm_connector_state *conn_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -431,7 +432,7 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 		break;
 	default:
 		MISSING_CASE(conn_state->scaling_mode);
-		return;
+		return -EINVAL;
 	}
 
 	/* 965+ wants fuzzy fitting */
@@ -452,6 +453,8 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 	crtc_state->gmch_pfit.control = pfit_control;
 	crtc_state->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
 	crtc_state->gmch_pfit.lvds_border_bits = border;
+
+	return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index e2fa1543a61f..a5c93232eb6f 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -25,10 +25,10 @@ int intel_panel_init(struct intel_panel *panel,
 void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
-void intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
+int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
+			    const struct drm_connector_state *conn_state);
+int intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
 			     const struct drm_connector_state *conn_state);
-void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
-			      const struct drm_connector_state *conn_state);
 void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
 				    u32 level, u32 max);
 int intel_panel_setup_backlight(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 97f72d320d87..485f2bbec530 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -278,9 +278,11 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
 		if (HAS_GMCH(dev_priv))
-			intel_gmch_panel_fitting(pipe_config, conn_state);
+			ret = intel_gmch_panel_fitting(pipe_config, conn_state);
 		else
-			intel_pch_panel_fitting(pipe_config, conn_state);
+			ret = intel_pch_panel_fitting(pipe_config, conn_state);
+		if (ret)
+			return ret;
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: pfit/scaler rework prep stuff (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: pfit/scaler rework prep stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/68409/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7926 -> Patchwork_16547
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_module_load@reload:
    - {fi-ehl-1}:         NOTRUN -> [WARN][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/fi-ehl-1/igt@i915_module_load@reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-n2820:       [PASS][2] -> [INCOMPLETE][3] ([i915#45])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][4] -> [FAIL][5] ([fdo#111407])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0:
    - {fi-ehl-1}:         [INCOMPLETE][6] ([i915#937]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/fi-ehl-1/igt@gem_exec_suspend@basic-s0.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/fi-ehl-1/igt@gem_exec_suspend@basic-s0.html

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

  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937


Participating hosts (45 -> 38)
------------------------------

  Additional (6): fi-skl-6770hq fi-bdw-gvtdvm fi-glk-dsi fi-gdg-551 fi-bsw-kefka fi-kbl-r 
  Missing    (13): fi-ilk-m540 fi-bdw-samus fi-bdw-5557u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-ivb-3770 fi-cfl-8109u fi-skl-6700k2 fi-blb-e6850 fi-byt-clapper fi-skl-6600u 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7926 -> Patchwork_16547

  CI-20190529: 20190529
  CI_DRM_7926: 6b2fe829d300abf285e9db8b252ffacd216df3ed @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5437: ae42fedfd0c536c560e8e17b06d9c7b94a4e8f0c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16547: 833bab58ec2fa453deb38303f5534118c328c5a5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

833bab58ec2f drm/i915: Have pfit calculations return an error code
6ea293f66e4d drm/i915: Pass connector state to pfit calculations
489d614c2d81 drm/i915: s/pipe_config/crtc_state/ in pfit functions
31b06d809c65 drm/i915: Use drm_rect to store the pfit window pos/size
29d781ef706b drm/i915: Flatten a bunch of the pfit functions
e78250ae53d0 drm/i915: Fix skl+ non-scaled pfit modes
0c04ecca2fc1 drm/i915: Use intel_de_write_fw() for skl+ scaler registers
0027eb6cbbdb drm/i915: Parametrize PFIT_PIPE

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: pfit/scaler rework prep stuff (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: pfit/scaler rework prep stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/68409/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7926_full -> Patchwork_16547_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-apl4/igt@gem_ctx_isolation@bcs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-apl6/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#112080]) +15 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb6/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#112146]) +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb3/igt@gem_exec_schedule@in-order-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb1/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@pi-shared-iova-bsd:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([i915#677])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb5/igt@gem_exec_schedule@pi-shared-iova-bsd.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb2/igt@gem_exec_schedule@pi-shared-iova-bsd.html

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109276]) +20 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb1/igt@gem_exec_schedule@preempt-contexts-bsd2.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb3/igt@gem_exec_schedule@preempt-contexts-bsd2.html

  * igt@gen7_exec_parse@basic-offset:
    - shard-hsw:          [PASS][11] -> [FAIL][12] ([i915#694])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-hsw6/igt@gen7_exec_parse@basic-offset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw1/igt@gen7_exec_parse@basic-offset.html

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-glk:          [PASS][13] -> [DMESG-WARN][14] ([i915#118] / [i915#95])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-glk6/igt@i915_pm_rpm@modeset-stress-extra-wait.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-glk8/igt@i915_pm_rpm@modeset-stress-extra-wait.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge:
    - shard-hsw:          [PASS][15] -> [INCOMPLETE][16] ([i915#61])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-hsw1/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw5/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][17] -> [FAIL][18] ([i915#79])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-kbl7/igt@kms_flip@flip-vs-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-kbl3/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([i915#49])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl2/igt@kms_frontbuffer_tracking@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl9/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-snb:          [PASS][23] -> [SKIP][24] ([fdo#109271])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-snb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109642] / [fdo#111068])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb3/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][29] -> [FAIL][30] ([i915#173])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb7/igt@kms_psr@no_drrs.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb7/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@perf_pmu@cpu-hotplug:
    - shard-hsw:          [PASS][33] -> [INCOMPLETE][34] ([i915#1176] / [i915#61])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-hsw7/igt@perf_pmu@cpu-hotplug.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw5/igt@perf_pmu@cpu-hotplug.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][35] ([fdo#110841]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][37] ([fdo#110854]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb3/igt@gem_exec_balancer@smoke.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb1/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@pi-userfault-bsd:
    - shard-iclb:         [SKIP][39] ([i915#677]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@gem_exec_schedule@pi-userfault-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb6/igt@gem_exec_schedule@pi-userfault-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [SKIP][41] ([fdo#109276]) -> [PASS][42] +21 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb5/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][43] ([fdo#112146]) -> [PASS][44] +7 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb3/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-kbl:          [FAIL][45] ([i915#644]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-kbl1/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-kbl2/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][47] ([i915#180]) -> [PASS][48] +3 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled:
    - shard-glk:          [FAIL][49] ([i915#52] / [i915#54]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-glk7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-glk5/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][51] ([i915#79]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl1/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          [FAIL][53] ([i915#34]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl6/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl10/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * {igt@kms_hdr@bpc-switch-suspend}:
    - shard-skl:          [FAIL][55] ([i915#1188]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl1/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +3 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][59] ([fdo#108145]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][61] ([fdo#109441]) -> [PASS][62] +2 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb5/igt@kms_psr@psr2_primary_page_flip.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  * igt@perf_pmu@busy-no-semaphores-vcs1:
    - shard-iclb:         [SKIP][63] ([fdo#112080]) -> [PASS][64] +10 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb8/igt@perf_pmu@busy-no-semaphores-vcs1.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb4/igt@perf_pmu@busy-no-semaphores-vcs1.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv-switch:
    - shard-iclb:         [FAIL][65] ([IGT#28]) -> [SKIP][66] ([fdo#112080])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html

  * igt@gem_tiled_blits@normal:
    - shard-hsw:          [FAIL][67] ([i915#818]) -> [FAIL][68] ([i915#694]) +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-hsw5/igt@gem_tiled_blits@normal.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw6/igt@gem_tiled_blits@normal.html

  * igt@runner@aborted:
    - shard-hsw:          [FAIL][69] ([i915#974]) -> ([FAIL][70], [FAIL][71]) ([i915#1176] / [i915#974])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7926/shard-hsw2/igt@runner@aborted.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw6/igt@runner@aborted.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/shard-hsw5/igt@runner@aborted.html

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

  [IGT#28]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/28
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#1176]: https://gitlab.freedesktop.org/drm/intel/issues/1176
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [i915#974]: https://gitlab.freedesktop.org/drm/intel/issues/974


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7926 -> Patchwork_16547

  CI-20190529: 20190529
  CI_DRM_7926: 6b2fe829d300abf285e9db8b252ffacd216df3ed @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5437: ae42fedfd0c536c560e8e17b06d9c7b94a4e8f0c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16547: 833bab58ec2fa453deb38303f5534118c328c5a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16547/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx