[PATCH 1/3] drm/i915: Nuke the cursor size defines

[PATCH 1/3] drm/i915: Nuke the cursor size defines

[Ville Syrjala]

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

No point in having this extra indireciton for the cursor max size.
So drop the defines and just write out the raw numbers. Makes it
easier to see what's going on.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++----
 drivers/gpu/drm/i915/intel_drv.h     | 6 ------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8251e189a8bb..f6655f482b67 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15054,11 +15054,11 @@ int intel_modeset_init(struct drm_device *dev)
 		dev->mode_config.cursor_width = IS_I845G(dev_priv) ? 64 : 512;
 		dev->mode_config.cursor_height = 1023;
 	} else if (IS_GEN2(dev_priv)) {
-		dev->mode_config.cursor_width = GEN2_CURSOR_WIDTH;
-		dev->mode_config.cursor_height = GEN2_CURSOR_HEIGHT;
+		dev->mode_config.cursor_width = 64;
+		dev->mode_config.cursor_height = 64;
 	} else {
-		dev->mode_config.cursor_width = MAX_CURSOR_WIDTH;
-		dev->mode_config.cursor_height = MAX_CURSOR_HEIGHT;
+		dev->mode_config.cursor_width = 256;
+		dev->mode_config.cursor_height = 256;
 	}
 
 	dev->mode_config.fb_base = ggtt->gmadr.start;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8840108749a5..2d09f08e5e0c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -158,12 +158,6 @@
 #define MAX_OUTPUTS 6
 /* maximum connectors per crtcs in the mode set */
 
-/* Maximum cursor sizes */
-#define GEN2_CURSOR_WIDTH 64
-#define GEN2_CURSOR_HEIGHT 64
-#define MAX_CURSOR_WIDTH 256
-#define MAX_CURSOR_HEIGHT 256
-
 #define INTEL_I2C_BUS_DVO 1
 #define INTEL_I2C_BUS_SDVO 2
 
-- 
2.16.4

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

[PATCH 2/3] drm/i915: Check timings against hardware maximums

[Ville Syrjala]

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

Validate that all display timings fit within the number of bits
we have in the transcoder timing registers.

The limits are:
hsw+:
 4k: vdisplay, vblank_start
 8k: everything else
gen3+:
 4k: h/vdisplay, h/vblank_start
 8k: everything else
gen2:
 2k: h/vdisplay, h/vblank_start
 4k: everything else

Also document the fact that the mode_config.max_width/height limits
refer to just the max framebuffer dimensions we support. Which may
be larger than the max hdisplay/vdisplay.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f6655f482b67..6e3aa6815b30 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14572,6 +14572,10 @@ static enum drm_mode_status
 intel_mode_valid(struct drm_device *dev,
 		 const struct drm_display_mode *mode)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	int hdisplay_max, htotal_max;
+	int vdisplay_max, vtotal_max;
+
 	/*
 	 * Can't reject DBLSCAN here because Xorg ddxen can add piles
 	 * of DBLSCAN modes to the output's mode list when they detect
@@ -14601,6 +14605,36 @@ intel_mode_valid(struct drm_device *dev,
 			   DRM_MODE_FLAG_CLKDIV2))
 		return MODE_BAD;
 
+	if (INTEL_GEN(dev_priv) >= 9 ||
+	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
+		hdisplay_max = 8192; /* FDI max 4096 handled elsewhere */
+		vdisplay_max = 4096;
+		htotal_max = 8192;
+		vtotal_max = 8192;
+	} else if (INTEL_GEN(dev_priv) >= 3) {
+		hdisplay_max = 4096;
+		vdisplay_max = 4096;
+		htotal_max = 8192;
+		vtotal_max = 8192;
+	} else {
+		hdisplay_max = 2048;
+		vdisplay_max = 2048;
+		htotal_max = 4096;
+		vtotal_max = 4096;
+	}
+
+	if (mode->hdisplay > hdisplay_max ||
+	    mode->hsync_start > htotal_max ||
+	    mode->hsync_end > htotal_max ||
+	    mode->htotal > htotal_max)
+		return MODE_H_ILLEGAL;
+
+	if (mode->vdisplay > vdisplay_max ||
+	    mode->vsync_start > vtotal_max ||
+	    mode->vsync_end > vtotal_max ||
+	    mode->vtotal > vtotal_max)
+		return MODE_V_ILLEGAL;
+
 	return MODE_OK;
 }
 
@@ -15039,6 +15073,7 @@ int intel_modeset_init(struct drm_device *dev)
 		}
 	}
 
+	/* maximum framebuffer dimensions */
 	if (IS_GEN2(dev_priv)) {
 		dev->mode_config.max_width = 2048;
 		dev->mode_config.max_height = 2048;
-- 
2.16.4

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

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Chris Wilson]

Quoting Ville Syrjala (2018-06-15 18:44:05)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Validate that all display timings fit within the number of bits
> we have in the transcoder timing registers.
> 
> The limits are:
> hsw+:
>  4k: vdisplay, vblank_start
>  8k: everything else
> gen3+:
>  4k: h/vdisplay, h/vblank_start
>  8k: everything else
> gen2:
>  2k: h/vdisplay, h/vblank_start
>  4k: everything else
> 
> Also document the fact that the mode_config.max_width/height limits
> refer to just the max framebuffer dimensions we support. Which may
> be larger than the max hdisplay/vdisplay.

In the ddx, I used them to filter max hdisplay/vdisplay... And
completely ignored them wrt to framebuffer.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Ville Syrjälä]

On Fri, Jun 15, 2018 at 07:44:08PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-06-15 18:44:05)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Validate that all display timings fit within the number of bits
> > we have in the transcoder timing registers.
> > 
> > The limits are:
> > hsw+:
> >  4k: vdisplay, vblank_start
> >  8k: everything else
> > gen3+:
> >  4k: h/vdisplay, h/vblank_start
> >  8k: everything else
> > gen2:
> >  2k: h/vdisplay, h/vblank_start
> >  4k: everything else
> > 
> > Also document the fact that the mode_config.max_width/height limits
> > refer to just the max framebuffer dimensions we support. Which may
> > be larger than the max hdisplay/vdisplay.
> 
> In the ddx, I used them to filter max hdisplay/vdisplay... And
> completely ignored them wrt to framebuffer.

Whatever works :)

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

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Chris Wilson]

Quoting Ville Syrjälä (2018-06-15 20:48:49)
> On Fri, Jun 15, 2018 at 07:44:08PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-06-15 18:44:05)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Validate that all display timings fit within the number of bits
> > > we have in the transcoder timing registers.
> > > 
> > > The limits are:
> > > hsw+:
> > >  4k: vdisplay, vblank_start
> > >  8k: everything else
> > > gen3+:
> > >  4k: h/vdisplay, h/vblank_start
> > >  8k: everything else
> > > gen2:
> > >  2k: h/vdisplay, h/vblank_start
> > >  4k: everything else
> > > 
> > > Also document the fact that the mode_config.max_width/height limits
> > > refer to just the max framebuffer dimensions we support. Which may
> > > be larger than the max hdisplay/vdisplay.
> > 
> > In the ddx, I used them to filter max hdisplay/vdisplay... And
> > completely ignored them wrt to framebuffer.
> 
> Whatever works :)

Yeah, and this doesn't break -intel afaict, since ultimately validation
is done by the kernel and we/the client just keeps on trying something
until it works (or more often until they just give up).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Ville Syrjälä]

On Fri, Jun 15, 2018 at 09:02:52PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-06-15 20:48:49)
> > On Fri, Jun 15, 2018 at 07:44:08PM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2018-06-15 18:44:05)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Validate that all display timings fit within the number of bits
> > > > we have in the transcoder timing registers.
> > > > 
> > > > The limits are:
> > > > hsw+:
> > > >  4k: vdisplay, vblank_start
> > > >  8k: everything else
> > > > gen3+:
> > > >  4k: h/vdisplay, h/vblank_start
> > > >  8k: everything else
> > > > gen2:
> > > >  2k: h/vdisplay, h/vblank_start
> > > >  4k: everything else
> > > > 
> > > > Also document the fact that the mode_config.max_width/height limits
> > > > refer to just the max framebuffer dimensions we support. Which may
> > > > be larger than the max hdisplay/vdisplay.
> > > 
> > > In the ddx, I used them to filter max hdisplay/vdisplay... And
> > > completely ignored them wrt to framebuffer.
> > 
> > Whatever works :)
> 
> Yeah, and this doesn't break -intel afaict, since ultimately validation
> is done by the kernel and we/the client just keeps on trying something
> until it works (or more often until they just give up).

Yeah. The main issue is the user being presented with a pile of modes
that can never actually work. It shouldn't be fatal but at least it's
annoying to the user.

I think we might want a new ioctl to have the kernel validate user
modes as well. I guess we could try to ressurect the old ioctls to
add/remove modes to the other list, but I'm thinking we might just want
something that takes the connector ID and a pile of modes and returns a
good/bad status for each (could snatch one of the mode type or flag bits
for that I suppose).

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

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Paulo Zanoni]

Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Validate that all display timings fit within the number of bits
> we have in the transcoder timing registers.
> 
> The limits are:
> hsw+:
>  4k: vdisplay, vblank_start
>  8k: everything else
> gen3+:
>  4k: h/vdisplay, h/vblank_start
>  8k: everything else
> gen2:
>  2k: h/vdisplay, h/vblank_start
>  4k: everything else
> 
> Also document the fact that the mode_config.max_width/height limits
> refer to just the max framebuffer dimensions we support. Which may
> be larger than the max hdisplay/vdisplay.

Verified against the specs. I can also confirm that the gen2+ specs
still exist at the old URLs :).

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

While at it, can't you try to implement the other restrictions listed
in those active/sync registers? Like hdisplay needs be multiples of 2
on gen2, hblank minimum being 32 on HSW (138 with audio), minimum
vblank veing 5 or 8, etc?

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 35
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f6655f482b67..6e3aa6815b30 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14572,6 +14572,10 @@ static enum drm_mode_status
>  intel_mode_valid(struct drm_device *dev,
>  		 const struct drm_display_mode *mode)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	int hdisplay_max, htotal_max;
> +	int vdisplay_max, vtotal_max;
> +
>  	/*
>  	 * Can't reject DBLSCAN here because Xorg ddxen can add
> piles
>  	 * of DBLSCAN modes to the output's mode list when they
> detect
> @@ -14601,6 +14605,36 @@ intel_mode_valid(struct drm_device *dev,
>  			   DRM_MODE_FLAG_CLKDIV2))
>  		return MODE_BAD;
>  
> +	if (INTEL_GEN(dev_priv) >= 9 ||
> +	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
> +		hdisplay_max = 8192; /* FDI max 4096 handled
> elsewhere */
> +		vdisplay_max = 4096;
> +		htotal_max = 8192;
> +		vtotal_max = 8192;
> +	} else if (INTEL_GEN(dev_priv) >= 3) {
> +		hdisplay_max = 4096;
> +		vdisplay_max = 4096;
> +		htotal_max = 8192;
> +		vtotal_max = 8192;
> +	} else {
> +		hdisplay_max = 2048;
> +		vdisplay_max = 2048;
> +		htotal_max = 4096;
> +		vtotal_max = 4096;
> +	}
> +
> +	if (mode->hdisplay > hdisplay_max ||
> +	    mode->hsync_start > htotal_max ||
> +	    mode->hsync_end > htotal_max ||
> +	    mode->htotal > htotal_max)
> +		return MODE_H_ILLEGAL;
> +
> +	if (mode->vdisplay > vdisplay_max ||
> +	    mode->vsync_start > vtotal_max ||
> +	    mode->vsync_end > vtotal_max ||
> +	    mode->vtotal > vtotal_max)
> +		return MODE_V_ILLEGAL;
> +
>  	return MODE_OK;
>  }
>  
> @@ -15039,6 +15073,7 @@ int intel_modeset_init(struct drm_device
> *dev)
>  		}
>  	}
>  
> +	/* maximum framebuffer dimensions */
>  	if (IS_GEN2(dev_priv)) {
>  		dev->mode_config.max_width = 2048;
>  		dev->mode_config.max_height = 2048;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Check timings against hardware maximums

[Ville Syrjälä]

On Fri, Jun 15, 2018 at 01:30:24PM -0700, Paulo Zanoni wrote:
> Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Validate that all display timings fit within the number of bits
> > we have in the transcoder timing registers.
> > 
> > The limits are:
> > hsw+:
> >  4k: vdisplay, vblank_start
> >  8k: everything else
> > gen3+:
> >  4k: h/vdisplay, h/vblank_start
> >  8k: everything else
> > gen2:
> >  2k: h/vdisplay, h/vblank_start
> >  4k: everything else
> > 
> > Also document the fact that the mode_config.max_width/height limits
> > refer to just the max framebuffer dimensions we support. Which may
> > be larger than the max hdisplay/vdisplay.
> 
> Verified against the specs. I can also confirm that the gen2+ specs
> still exist at the old URLs :).

Cool. Thanks for digging through them.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> While at it, can't you try to implement the other restrictions listed
> in those active/sync registers? Like hdisplay needs be multiples of 2
> on gen2, hblank minimum being 32 on HSW (138 with audio), minimum
> vblank veing 5 or 8, etc?

I did more or less collect up the minimum/alignment restrictions as
well, but wasn't really convinced checking for them is all that useful.
There are probably some we should check like the HDMI/audio
hblank/front porch restrictions, and if those aren't met we should
probably fall back to driving the link on DVI mode or w/o audio.

If we do check all the minimums then we'd have to decide whether to
check against the absolute minimum from the spec for each platforms
(which would end up being somewhat messy on account of the limits having
changed so many times) or if we should just simplify a bit and declare
eg. a minimum of 8 (or even higher) for hdisplay/vdisplay across the
board.

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 35
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index f6655f482b67..6e3aa6815b30 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14572,6 +14572,10 @@ static enum drm_mode_status
> >  intel_mode_valid(struct drm_device *dev,
> >  		 const struct drm_display_mode *mode)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	int hdisplay_max, htotal_max;
> > +	int vdisplay_max, vtotal_max;
> > +
> >  	/*
> >  	 * Can't reject DBLSCAN here because Xorg ddxen can add
> > piles
> >  	 * of DBLSCAN modes to the output's mode list when they
> > detect
> > @@ -14601,6 +14605,36 @@ intel_mode_valid(struct drm_device *dev,
> >  			   DRM_MODE_FLAG_CLKDIV2))
> >  		return MODE_BAD;
> >  
> > +	if (INTEL_GEN(dev_priv) >= 9 ||
> > +	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
> > +		hdisplay_max = 8192; /* FDI max 4096 handled
> > elsewhere */
> > +		vdisplay_max = 4096;
> > +		htotal_max = 8192;
> > +		vtotal_max = 8192;
> > +	} else if (INTEL_GEN(dev_priv) >= 3) {
> > +		hdisplay_max = 4096;
> > +		vdisplay_max = 4096;
> > +		htotal_max = 8192;
> > +		vtotal_max = 8192;
> > +	} else {
> > +		hdisplay_max = 2048;
> > +		vdisplay_max = 2048;
> > +		htotal_max = 4096;
> > +		vtotal_max = 4096;
> > +	}
> > +
> > +	if (mode->hdisplay > hdisplay_max ||
> > +	    mode->hsync_start > htotal_max ||
> > +	    mode->hsync_end > htotal_max ||
> > +	    mode->htotal > htotal_max)
> > +		return MODE_H_ILLEGAL;
> > +
> > +	if (mode->vdisplay > vdisplay_max ||
> > +	    mode->vsync_start > vtotal_max ||
> > +	    mode->vsync_end > vtotal_max ||
> > +	    mode->vtotal > vtotal_max)
> > +		return MODE_V_ILLEGAL;
> > +
> >  	return MODE_OK;
> >  }
> >  
> > @@ -15039,6 +15073,7 @@ int intel_modeset_init(struct drm_device
> > *dev)
> >  		}
> >  	}
> >  
> > +	/* maximum framebuffer dimensions */
> >  	if (IS_GEN2(dev_priv)) {
> >  		dev->mode_config.max_width = 2048;
> >  		dev->mode_config.max_height = 2048;

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

[PATCH 3/3] drm/i915: Enforce max hdisplay/hblank_start limits on HSW/BDW FDI

[Ville Syrjala]

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

The PCH transcoder registers are only 12 bits wide for the hdisplay
and hblank_start values. On HSW/BDW the CPU side registers are 13
bits wide. intel_mode_valid() only checks against the higher limit
(since we don't know where the mode is to be used), so an extra
check is required against the FDI limits.

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

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 95aa29cf2d9c..457b1a2d05b8 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -333,6 +333,10 @@ intel_crt_mode_valid(struct drm_connector *connector,
 	    (ironlake_get_lanes_required(mode->clock, 270000, 24) > 2))
 		return MODE_CLOCK_HIGH;
 
+	/* HSW/BDW FDI limited to 4k */
+	if (mode->hdisplay > 4096)
+		return MODE_H_ILLEGAL;
+
 	return MODE_OK;
 }
 
@@ -375,6 +379,11 @@ static bool hsw_crt_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return false;
 
+	/* HSW/BDW FDI limited to 4k */
+	if (adjusted_mode->crtc_hdisplay > 4096 ||
+	    adjusted_mode->crtc_hblank_start > 4096)
+		return false;
+
 	pipe_config->has_pch_encoder = true;
 
 	/* LPT FDI RX only supports 8bpc. */
-- 
2.16.4

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

Re: [PATCH 3/3] drm/i915: Enforce max hdisplay/hblank_start limits on HSW/BDW FDI

[Paulo Zanoni]

Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The PCH transcoder registers are only 12 bits wide for the hdisplay
> and hblank_start values. On HSW/BDW the CPU side registers are 13
> bits wide. intel_mode_valid() only checks against the higher limit
> (since we don't know where the mode is to be used), so an extra
> check is required against the FDI limits.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c
> b/drivers/gpu/drm/i915/intel_crt.c
> index 95aa29cf2d9c..457b1a2d05b8 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -333,6 +333,10 @@ intel_crt_mode_valid(struct drm_connector
> *connector,
>  	    (ironlake_get_lanes_required(mode->clock, 270000, 24) >
> 2))
>  		return MODE_CLOCK_HIGH;
>  
> +	/* HSW/BDW FDI limited to 4k */
> +	if (mode->hdisplay > 4096)
> +		return MODE_H_ILLEGAL;
> +
>  	return MODE_OK;
>  }
>  
> @@ -375,6 +379,11 @@ static bool hsw_crt_compute_config(struct
> intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return false;
>  
> +	/* HSW/BDW FDI limited to 4k */
> +	if (adjusted_mode->crtc_hdisplay > 4096 ||
> +	    adjusted_mode->crtc_hblank_start > 4096)
> +		return false;

Meh, doubling checks is not cool.

By the way, doesn't this chunk make more sense inside
ironlake_fdi_compute_config()?

Just to make sure: the only cases that could escape mode_valid() and
make it into compute_config() are for panel fitting, right?

With or without changes:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thanks,
Paulo 

> +
>  	pipe_config->has_pch_encoder = true;
>  
>  	/* LPT FDI RX only supports 8bpc. */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Enforce max hdisplay/hblank_start limits on HSW/BDW FDI

[Ville Syrjälä]

On Fri, Jun 15, 2018 at 02:09:12PM -0700, Paulo Zanoni wrote:
> Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The PCH transcoder registers are only 12 bits wide for the hdisplay
> > and hblank_start values. On HSW/BDW the CPU side registers are 13
> > bits wide. intel_mode_valid() only checks against the higher limit
> > (since we don't know where the mode is to be used), so an extra
> > check is required against the FDI limits.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_crt.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c
> > b/drivers/gpu/drm/i915/intel_crt.c
> > index 95aa29cf2d9c..457b1a2d05b8 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -333,6 +333,10 @@ intel_crt_mode_valid(struct drm_connector
> > *connector,
> >  	    (ironlake_get_lanes_required(mode->clock, 270000, 24) >
> > 2))
> >  		return MODE_CLOCK_HIGH;
> >  
> > +	/* HSW/BDW FDI limited to 4k */
> > +	if (mode->hdisplay > 4096)
> > +		return MODE_H_ILLEGAL;
> > +
> >  	return MODE_OK;
> >  }
> >  
> > @@ -375,6 +379,11 @@ static bool hsw_crt_compute_config(struct
> > intel_encoder *encoder,
> >  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >  		return false;
> >  
> > +	/* HSW/BDW FDI limited to 4k */
> > +	if (adjusted_mode->crtc_hdisplay > 4096 ||
> > +	    adjusted_mode->crtc_hblank_start > 4096)
> > +		return false;
> 
> Meh, doubling checks is not cool.
> 
> By the way, doesn't this chunk make more sense inside
> ironlake_fdi_compute_config()?

Hmm. I don't see a particularly good reason for putting it there. All
that guy is doing is computing the number of lanes and the M/N values.
Nothing to do with transcoder timings. It's also called fairly late in
the process, so we'd end up doing quite a bit of pointless work that
way.

> 
> Just to make sure: the only cases that could escape mode_valid() and
> make it into compute_config() are for panel fitting, right?

The connector .mode_valid() won't be called for setcrtc/atomic
ioctls at all.

> 
> With or without changes:
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Thanks,
> Paulo 
> 
> > +
> >  	pipe_config->has_pch_encoder = true;
> >  
> >  	/* LPT FDI RX only supports 8bpc. */

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

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Nuke the cursor size defines

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the cursor size defines
URL   : https://patchwork.freedesktop.org/series/44854/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4325 -> Patchwork_9334 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44854/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#100368)

    igt@kms_setmode@basic-clone-single-crtc:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106238)

    
    ==== Possible fixes ====

    igt@kms_flip@basic-plain-flip:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS +1

    igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
      fi-glk-j4005:       DMESG-WARN (fdo#106000, fdo#106097) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106238 https://bugs.freedesktop.org/show_bug.cgi?id=106238


== Participating hosts (43 -> 38) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4325 -> Patchwork_9334

  CI_DRM_4325: 4275ebe85ad179007c49b7bcf78d340b7681871e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4520: 91f5d4665b07f073c78abd3cd4b8e0e347dbf638 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9334: 82fa25eaf0c5b6d63d5496ad94c00d58032586ab @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

82fa25eaf0c5 drm/i915: Enforce max hdisplay/hblank_start limits on HSW/BDW FDI
ac80851f6ceb drm/i915: Check timings against hardware maximums
6f1e10daa90f drm/i915: Nuke the cursor size defines

== Logs ==

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

Re: [PATCH 1/3] drm/i915: Nuke the cursor size defines

[Paulo Zanoni]

Em Sex, 2018-06-15 às 20:44 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No point in having this extra indireciton for the cursor max size.
> So drop the defines and just write out the raw numbers. Makes it
> easier to see what's going on.

And the gen2 definition doesn't even say "max" and the other similar
parts of the code such as buffer max width/height don't use
definitions.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++----
>  drivers/gpu/drm/i915/intel_drv.h     | 6 ------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8251e189a8bb..f6655f482b67 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15054,11 +15054,11 @@ int intel_modeset_init(struct drm_device
> *dev)
>  		dev->mode_config.cursor_width = IS_I845G(dev_priv) ?
> 64 : 512;
>  		dev->mode_config.cursor_height = 1023;
>  	} else if (IS_GEN2(dev_priv)) {
> -		dev->mode_config.cursor_width = GEN2_CURSOR_WIDTH;
> -		dev->mode_config.cursor_height = GEN2_CURSOR_HEIGHT;
> +		dev->mode_config.cursor_width = 64;
> +		dev->mode_config.cursor_height = 64;
>  	} else {
> -		dev->mode_config.cursor_width = MAX_CURSOR_WIDTH;
> -		dev->mode_config.cursor_height = MAX_CURSOR_HEIGHT;
> +		dev->mode_config.cursor_width = 256;
> +		dev->mode_config.cursor_height = 256;
>  	}
>  
>  	dev->mode_config.fb_base = ggtt->gmadr.start;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 8840108749a5..2d09f08e5e0c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -158,12 +158,6 @@
>  #define MAX_OUTPUTS 6
>  /* maximum connectors per crtcs in the mode set */
>  
> -/* Maximum cursor sizes */
> -#define GEN2_CURSOR_WIDTH 64
> -#define GEN2_CURSOR_HEIGHT 64
> -#define MAX_CURSOR_WIDTH 256
> -#define MAX_CURSOR_HEIGHT 256
> -
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915: Nuke the cursor size defines

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the cursor size defines
URL   : https://patchwork.freedesktop.org/series/44854/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4325_full -> Patchwork_9334_full =

== Summary - FAILURE ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@drv_selftest@live_execlists:
      shard-apl:          SKIP -> PASS +1

    igt@gem_exec_schedule@deep-render:
      shard-kbl:          SKIP -> PASS

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          PASS -> SKIP +1

    igt@kms_frontbuffer_tracking@basic:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          PASS -> FAIL (fdo#105347)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-pgflip-blt:
      shard-glk:          PASS -> FAIL (fdo#103167, fdo#104724)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-glk:          FAIL (fdo#105347) -> PASS

    igt@drv_selftest@live_hangcheck:
      shard-apl:          DMESG-FAIL -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#104724) -> PASS

    igt@kms_vblank@pipe-c-accuracy-idle:
      shard-glk:          FAIL (fdo#102583) -> PASS

    
  fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4325 -> Patchwork_9334

  CI_DRM_4325: 4275ebe85ad179007c49b7bcf78d340b7681871e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4520: 91f5d4665b07f073c78abd3cd4b8e0e347dbf638 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9334: 82fa25eaf0c5b6d63d5496ad94c00d58032586ab @ 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_9334/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Nuke the cursor size defines

[Rodrigo Vivi]

On Fri, Jun 15, 2018 at 08:44:04PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No point in having this extra indireciton for the cursor max size.
> So drop the defines and just write out the raw numbers. Makes it
> easier to see what's going on.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++----
>  drivers/gpu/drm/i915/intel_drv.h     | 6 ------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8251e189a8bb..f6655f482b67 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15054,11 +15054,11 @@ int intel_modeset_init(struct drm_device *dev)
>  		dev->mode_config.cursor_width = IS_I845G(dev_priv) ? 64 : 512;
>  		dev->mode_config.cursor_height = 1023;
>  	} else if (IS_GEN2(dev_priv)) {
> -		dev->mode_config.cursor_width = GEN2_CURSOR_WIDTH;
> -		dev->mode_config.cursor_height = GEN2_CURSOR_HEIGHT;
> +		dev->mode_config.cursor_width = 64;
> +		dev->mode_config.cursor_height = 64;
>  	} else {
> -		dev->mode_config.cursor_width = MAX_CURSOR_WIDTH;
> -		dev->mode_config.cursor_height = MAX_CURSOR_HEIGHT;
> +		dev->mode_config.cursor_width = 256;
> +		dev->mode_config.cursor_height = 256;
>  	}
>  
>  	dev->mode_config.fb_base = ggtt->gmadr.start;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8840108749a5..2d09f08e5e0c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -158,12 +158,6 @@
>  #define MAX_OUTPUTS 6
>  /* maximum connectors per crtcs in the mode set */
>  
> -/* Maximum cursor sizes */
> -#define GEN2_CURSOR_WIDTH 64
> -#define GEN2_CURSOR_HEIGHT 64
> -#define MAX_CURSOR_WIDTH 256
> -#define MAX_CURSOR_HEIGHT 256
> -
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> -- 
> 2.16.4
> 
> _______________________________________________
> 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