[PATCH 0/8] Skylake primary plane read-out

[PATCH 0/8] Skylake primary plane read-out

[Damien Lespiau]

Skylake changed a few things here and there in the plane registers and we
weren't correctly reading out the primary plane state when trying to re-use the
BIOS stolen allocated fb (especially the stride was all wrong). Of course,
weird artefacts ensued when loading the driver.

This series implement get_plane_config() for SKL, preceded  by a few clean-up
patches. No artefacts when loading the driver anymore, at least on this end.

Damien Lespiau (8):
  drm/i915: Change plane_config to store a tiling_mode
  drm/i915: Use a common function for computing the fb height alignment
  drm/i915: Unclutter the get_plane() functions
  drm/i915: Don't use crtc->plane in ILK+ get_config()
  drm/i915: Use pipe_name() in the get_plane_config() functions
  drm/i915: Make intel_format_to_fourcc() static
  drm/i915/skl: intel_format_to_fourcc() doesn't work for SKL planes
  drm/i915/skl: Provide a Skylake version of get_plane_config()

 drivers/gpu/drm/i915/intel_display.c | 213 +++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |   5 +-
 drivers/gpu/drm/i915/intel_fbdev.c   |   3 +-
 3 files changed, 157 insertions(+), 64 deletions(-)

-- 
1.8.3.1

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

[PATCH 1/8] drm/i915: Change plane_config to store a tiling_mode

[Damien Lespiau]

Rather than having "tiled" meaning "is it X-tiled?" convert the field to
explicitely store the tiling mode. The code doesn't have to change much
as 1 is conveniently I915_TILING_X.

This is to accommodate future changes around tiling modes and scannout
buffers.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 ++++++++---------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_fbdev.c   |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92f0005..34672e8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2329,10 +2329,9 @@ static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
 	if (!obj)
 		return false;
 
-	if (plane_config->tiled) {
-		obj->tiling_mode = I915_TILING_X;
+	obj->tiling_mode = plane_config->tiling;
+	if (obj->tiling_mode == I915_TILING_X)
 		obj->stride = crtc->base.primary->fb->pitches[0];
-	}
 
 	mode_cmd.pixel_format = crtc->base.primary->fb->pixel_format;
 	mode_cmd.width = crtc->base.primary->fb->width;
@@ -6470,7 +6469,7 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 
 	if (INTEL_INFO(dev)->gen >= 4)
 		if (val & DISPPLANE_TILED)
-			plane_config->tiled = true;
+			plane_config->tiling = I915_TILING_X;
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
@@ -6479,7 +6478,7 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 		drm_format_plane_cpp(fourcc, 0) * 8;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
-		if (plane_config->tiled)
+		if (plane_config->tiling)
 			offset = I915_READ(DSPTILEOFF(plane));
 		else
 			offset = I915_READ(DSPLINOFF(plane));
@@ -6497,7 +6496,7 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
 
 	aligned_height = intel_align_height(dev, crtc->base.primary->fb->height,
-					    plane_config->tiled);
+					    plane_config->tiling);
 
 	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
 					aligned_height);
@@ -7502,7 +7501,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	if (INTEL_INFO(dev)->gen >= 4)
 		if (val & DISPPLANE_TILED)
-			plane_config->tiled = true;
+			plane_config->tiling = I915_TILING_X;
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
@@ -7514,7 +7513,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		offset = I915_READ(DSPOFFSET(plane));
 	} else {
-		if (plane_config->tiled)
+		if (plane_config->tiling)
 			offset = I915_READ(DSPTILEOFF(plane));
 		else
 			offset = I915_READ(DSPLINOFF(plane));
@@ -7529,7 +7528,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
 
 	aligned_height = intel_align_height(dev, crtc->base.primary->fb->height,
-					    plane_config->tiled);
+					    plane_config->tiling);
 
 	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
 					aligned_height);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53ac23..cb431db 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -255,7 +255,7 @@ struct intel_plane_state {
 };
 
 struct intel_plane_config {
-	bool tiled;
+	unsigned int tiling;
 	int size;
 	u32 base;
 };
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 9b584f3..1805012 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -575,7 +575,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 		}
 
 		cur_size = intel_crtc->config.adjusted_mode.crtc_vdisplay;
-		cur_size = ALIGN(cur_size, plane_config->tiled ? (IS_GEN2(dev) ? 16 : 8) : 1);
+		cur_size = ALIGN(cur_size, plane_config->tiling ? (IS_GEN2(dev) ? 16 : 8) : 1);
 		cur_size *= fb->base.pitches[0];
 		DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n",
 			      pipe_name(intel_crtc->pipe),
-- 
1.8.3.1

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

[PATCH 2/8] drm/i915: Use a common function for computing the fb height alignment

[Damien Lespiau]

If we need to change the fb height constraints, it sounds like a good
idea to have to do it in one place only.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_fbdev.c   |  3 ++-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 34672e8..2a84e47 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2167,11 +2167,12 @@ static bool need_vtd_wa(struct drm_device *dev)
 	return false;
 }
 
-static int intel_align_height(struct drm_device *dev, int height, bool tiled)
+int
+intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling)
 {
 	int tile_height;
 
-	tile_height = tiled ? (IS_GEN2(dev) ? 16 : 8) : 1;
+	tile_height = tiling ? (IS_GEN2(dev) ? 16 : 8) : 1;
 	return ALIGN(height, tile_height);
 }
 
@@ -6495,8 +6496,9 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	val = I915_READ(DSPSTRIDE(pipe));
 	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_align_height(dev, crtc->base.primary->fb->height,
-					    plane_config->tiling);
+	aligned_height = intel_fb_align_height(dev,
+					       crtc->base.primary->fb->height,
+					       plane_config->tiling);
 
 	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
 					aligned_height);
@@ -7527,8 +7529,9 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	val = I915_READ(DSPSTRIDE(pipe));
 	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_align_height(dev, crtc->base.primary->fb->height,
-					    plane_config->tiling);
+	aligned_height = intel_fb_align_height(dev,
+					       crtc->base.primary->fb->height,
+					       plane_config->tiling);
 
 	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
 					aligned_height);
@@ -12277,8 +12280,8 @@ static int intel_framebuffer_init(struct drm_device *dev,
 	if (mode_cmd->offsets[0] != 0)
 		return -EINVAL;
 
-	aligned_height = intel_align_height(dev, mode_cmd->height,
-					    obj->tiling_mode);
+	aligned_height = intel_fb_align_height(dev, mode_cmd->height,
+					       obj->tiling_mode);
 	/* FIXME drm helper for size checks (especially planar formats)? */
 	if (obj->base.size < aligned_height * mode_cmd->pitches[0])
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb431db..e8235ae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -848,6 +848,8 @@ void intel_frontbuffer_flip(struct drm_device *dev,
 	intel_frontbuffer_flush(dev, frontbuffer_bits);
 }
 
+int intel_fb_align_height(struct drm_device *dev, int height,
+			  unsigned int tiling);
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
 
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 1805012..66cde1e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -575,7 +575,8 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 		}
 
 		cur_size = intel_crtc->config.adjusted_mode.crtc_vdisplay;
-		cur_size = ALIGN(cur_size, plane_config->tiling ? (IS_GEN2(dev) ? 16 : 8) : 1);
+		cur_size = intel_fb_align_height(dev, cur_size,
+						 plane_config->tiling);
 		cur_size *= fb->base.pitches[0];
 		DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n",
 			      pipe_name(intel_crtc->pipe),
-- 
1.8.3.1

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

[PATCH 3/8] drm/i915: Unclutter the get_plane() functions

[Damien Lespiau]

crtc->base.primary->fb was used everywhere. Use fb to temporarily point
there and don't forget to assign fb to its final destination at the end.

v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a84e47..6c042eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6459,9 +6459,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	int pipe = crtc->pipe, plane = crtc->plane;
 	int fourcc, pixel_format;
 	int aligned_height;
+	struct drm_framebuffer *fb;
 
-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
-	if (!crtc->base.primary->fb) {
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -6474,9 +6475,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	crtc->base.primary->fb->pixel_format = fourcc;
-	crtc->base.primary->fb->bits_per_pixel =
-		drm_format_plane_cpp(fourcc, 0) * 8;
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
 		if (plane_config->tiling)
@@ -6490,26 +6490,22 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	crtc->base.primary->fb->width = ((val >> 16) & 0xfff) + 1;
-	crtc->base.primary->fb->height = ((val >> 0) & 0xfff) + 1;
+	fb->width = ((val >> 16) & 0xfff) + 1;
+	fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
+	fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_fb_align_height(dev,
-					       crtc->base.primary->fb->height,
+	aligned_height = intel_fb_align_height(dev, fb->height,
 					       plane_config->tiling);
 
-	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
-					aligned_height);
+	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, crtc->base.primary->fb->width,
-		      crtc->base.primary->fb->height,
-		      crtc->base.primary->fb->bits_per_pixel, base,
-		      crtc->base.primary->fb->pitches[0],
-		      plane_config->size);
+		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
+		      base, fb->pitches[0], plane_config->size);
 
+	crtc->base.primary->fb = fb;
 }
 
 static void chv_crtc_clock_get(struct intel_crtc *crtc,
@@ -7492,9 +7488,10 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	int pipe = crtc->pipe, plane = crtc->plane;
 	int fourcc, pixel_format;
 	int aligned_height;
+	struct drm_framebuffer *fb;
 
-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
-	if (!crtc->base.primary->fb) {
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
 		return;
 	}
@@ -7507,9 +7504,8 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
 	fourcc = intel_format_to_fourcc(pixel_format);
-	crtc->base.primary->fb->pixel_format = fourcc;
-	crtc->base.primary->fb->bits_per_pixel =
-		drm_format_plane_cpp(fourcc, 0) * 8;
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
 	base = I915_READ(DSPSURF(plane)) & 0xfffff000;
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
@@ -7523,25 +7519,22 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	plane_config->base = base;
 
 	val = I915_READ(PIPESRC(pipe));
-	crtc->base.primary->fb->width = ((val >> 16) & 0xfff) + 1;
-	crtc->base.primary->fb->height = ((val >> 0) & 0xfff) + 1;
+	fb->width = ((val >> 16) & 0xfff) + 1;
+	fb->height = ((val >> 0) & 0xfff) + 1;
 
 	val = I915_READ(DSPSTRIDE(pipe));
-	crtc->base.primary->fb->pitches[0] = val & 0xffffffc0;
+	fb->pitches[0] = val & 0xffffffc0;
 
-	aligned_height = intel_fb_align_height(dev,
-					       crtc->base.primary->fb->height,
+	aligned_height = intel_fb_align_height(dev, fb->height,
 					       plane_config->tiling);
 
-	plane_config->size = PAGE_ALIGN(crtc->base.primary->fb->pitches[0] *
-					aligned_height);
+	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
 	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, crtc->base.primary->fb->width,
-		      crtc->base.primary->fb->height,
-		      crtc->base.primary->fb->bits_per_pixel, base,
-		      crtc->base.primary->fb->pitches[0],
-		      plane_config->size);
+		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
+		      base, fb->pitches[0], plane_config->size);
+
+	crtc->base.primary->fb = fb;
 }
 
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
-- 
1.8.3.1

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

[PATCH 4/8] drm/i915: Don't use crtc->plane in ILK+ get_config()

[Damien Lespiau]

crtc->plane can only be different from crtc->pipe pre-Gen4. Don't use it
in new-ish code.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6c042eb..ec3b90f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7485,7 +7485,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 val, base, offset;
-	int pipe = crtc->pipe, plane = crtc->plane;
+	int pipe = crtc->pipe;
 	int fourcc, pixel_format;
 	int aligned_height;
 	struct drm_framebuffer *fb;
@@ -7496,7 +7496,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 		return;
 	}
 
-	val = I915_READ(DSPCNTR(plane));
+	val = I915_READ(DSPCNTR(pipe));
 
 	if (INTEL_INFO(dev)->gen >= 4)
 		if (val & DISPPLANE_TILED)
@@ -7507,14 +7507,14 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 	fb->pixel_format = fourcc;
 	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
-	base = I915_READ(DSPSURF(plane)) & 0xfffff000;
+	base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
-		offset = I915_READ(DSPOFFSET(plane));
+		offset = I915_READ(DSPOFFSET(pipe));
 	} else {
 		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(plane));
+			offset = I915_READ(DSPTILEOFF(pipe));
 		else
-			offset = I915_READ(DSPLINOFF(plane));
+			offset = I915_READ(DSPLINOFF(pipe));
 	}
 	plane_config->base = base;
 
@@ -7530,8 +7530,8 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
-	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
+	DRM_DEBUG_KMS("pipe %d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      pipe, fb->width, fb->height, fb->bits_per_pixel,
 		      base, fb->pitches[0], plane_config->size);
 
 	crtc->base.primary->fb = fb;
-- 
1.8.3.1

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

[PATCH 5/8] drm/i915: Use pipe_name() in the get_plane_config() functions

[Damien Lespiau]

We may as well try to be consistent everywhere and know the pipes by
their name.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3b90f..200a62e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6501,9 +6501,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 
 	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
-	DRM_DEBUG_KMS("pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, plane, fb->width, fb->height, fb->bits_per_pixel,
-		      base, fb->pitches[0], plane_config->size);
+	DRM_DEBUG_KMS("pipe/plane %c/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      pipe_name(pipe), plane, fb->width, fb->height,
+		      fb->bits_per_pixel, base, fb->pitches[0],
+		      plane_config->size);
 
 	crtc->base.primary->fb = fb;
 }
@@ -7530,9 +7531,10 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 
 	plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
 
-	DRM_DEBUG_KMS("pipe %d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe, fb->width, fb->height, fb->bits_per_pixel,
-		      base, fb->pitches[0], plane_config->size);
+	DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      pipe_name(pipe), fb->width, fb->height,
+		      fb->bits_per_pixel, base, fb->pitches[0],
+		      plane_config->size);
 
 	crtc->base.primary->fb = fb;
 }
-- 
1.8.3.1

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

[PATCH 6/8] drm/i915: Make intel_format_to_fourcc() static

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 drivers/gpu/drm/i915/intel_drv.h     | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 200a62e..c77cf9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2293,7 +2293,7 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 	}
 }
 
-int intel_format_to_fourcc(int format)
+static int intel_format_to_fourcc(int format)
 {
 	switch (format) {
 	case DISPPLANE_8BPP:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e8235ae..f0b30d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -959,7 +959,6 @@ enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_config *pipe_config);
-int intel_format_to_fourcc(int format);
 void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
 void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
 
-- 
1.8.3.1

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

[PATCH 7/8] drm/i915/skl: intel_format_to_fourcc() doesn't work for SKL planes

[Damien Lespiau]

We will have a skl_ version shortly!

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c77cf9b..02b2a97 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2293,7 +2293,7 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 	}
 }
 
-static int intel_format_to_fourcc(int format)
+static int i9xx_format_to_fourcc(int format)
 {
 	switch (format) {
 	case DISPPLANE_8BPP:
@@ -6474,7 +6474,7 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
-	fourcc = intel_format_to_fourcc(pixel_format);
+	fourcc = i9xx_format_to_fourcc(pixel_format);
 	fb->pixel_format = fourcc;
 	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
@@ -7504,7 +7504,7 @@ static void ironlake_get_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
-	fourcc = intel_format_to_fourcc(pixel_format);
+	fourcc = i9xx_format_to_fourcc(pixel_format);
 	fb->pixel_format = fourcc;
 	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
 
-- 
1.8.3.1

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

[PATCH 8/8] drm/i915/skl: Provide a Skylake version of get_plane_config()

[Damien Lespiau]

Universal planes have changed a bit the register organization.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 108 ++++++++++++++++++++++++++++++++---
 1 file changed, 101 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 02b2a97..33e8112 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2314,6 +2314,32 @@ static int i9xx_format_to_fourcc(int format)
 	}
 }
 
+static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
+{
+	switch (format) {
+	case PLANE_CTL_FORMAT_RGB_565:
+		return DRM_FORMAT_RGB565;
+	default:
+	case PLANE_CTL_FORMAT_XRGB_8888:
+		if (rgb_order) {
+			if (alpha)
+				return DRM_FORMAT_ABGR8888;
+			else
+				return DRM_FORMAT_XBGR8888;
+		} else {
+			if (alpha)
+				return DRM_FORMAT_ARGB8888;
+			else
+				return DRM_FORMAT_XRGB8888;
+		}
+	case PLANE_CTL_FORMAT_XRGB_2101010:
+		if (rgb_order)
+			return DRM_FORMAT_XBGR2101010;
+		else
+			return DRM_FORMAT_XRGB2101010;
+	}
+}
+
 static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
 				  struct intel_plane_config *plane_config)
 {
@@ -7456,6 +7482,69 @@ static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
 				     &pipe_config->fdi_m_n, NULL);
 }
 
+static void skylake_get_plane_config(struct intel_crtc *crtc,
+				      struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, base, offset, stride_mult;
+	int pipe = crtc->pipe;
+	int fourcc, pixel_format;
+	int aligned_height;
+	struct drm_framebuffer *fb;
+
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
+		DRM_DEBUG_KMS("failed to alloc fb\n");
+		return;
+	}
+
+	val = I915_READ(PLANE_CTL(pipe, 0));
+	if (val & PLANE_CTL_TILED_MASK)
+		plane_config->tiling = I915_TILING_X;
+
+	pixel_format = val & PLANE_CTL_FORMAT_MASK;
+	fourcc = skl_format_to_fourcc(pixel_format,
+				      val & PLANE_CTL_ORDER_RGBX,
+				      val & PLANE_CTL_ALPHA_MASK);
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
+
+	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
+	plane_config->base = base;
+
+	offset = I915_READ(PLANE_OFFSET(pipe, 0));
+
+	val = I915_READ(PIPESRC(pipe));
+	fb->width = ((val >> 16) & 0xfff) + 1;
+	fb->height = ((val >> 0) & 0xfff) + 1;
+
+	val = I915_READ(PLANE_STRIDE(pipe, 0));
+	switch (plane_config->tiling) {
+	case I915_TILING_NONE:
+		stride_mult = 64;
+		break;
+	case I915_TILING_X:
+		stride_mult = 512;
+		break;
+	default:
+		BUG();
+	}
+	fb->pitches[0] = (val & 0x3ff) * stride_mult;
+
+	aligned_height = intel_fb_align_height(dev, fb->height,
+					       plane_config->tiling);
+
+	plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE);
+
+	DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      pipe_name(pipe), fb->width, fb->height,
+		      fb->bits_per_pixel, base, fb->pitches[0],
+		      plane_config->size);
+
+	crtc->base.primary->fb = fb;
+}
+
 static void ironlake_get_pfit_config(struct intel_crtc *crtc,
 				     struct intel_crtc_config *pipe_config)
 {
@@ -12336,19 +12425,24 @@ static void intel_init_display(struct drm_device *dev)
 	else
 		dev_priv->display.find_dpll = i9xx_find_best_dpll;
 
-	if (HAS_DDI(dev)) {
+	if (INTEL_INFO(dev)->gen >= 9) {
+		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
+		dev_priv->display.get_plane_config = skylake_get_plane_config;
+		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
+		dev_priv->display.crtc_enable = haswell_crtc_enable;
+		dev_priv->display.crtc_disable = haswell_crtc_disable;
+		dev_priv->display.off = ironlake_crtc_off;
+		dev_priv->display.update_primary_plane =
+			skylake_update_primary_plane;
+	} else if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_plane_config = ironlake_get_plane_config;
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
 		dev_priv->display.off = ironlake_crtc_off;
-		if (INTEL_INFO(dev)->gen >= 9)
-			dev_priv->display.update_primary_plane =
-				skylake_update_primary_plane;
-		else
-			dev_priv->display.update_primary_plane =
-				ironlake_update_primary_plane;
+		dev_priv->display.update_primary_plane =
+			ironlake_update_primary_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.get_plane_config = ironlake_get_plane_config;
-- 
1.8.3.1

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

Re: [PATCH 3/8] drm/i915: Unclutter the get_plane() functions

[Tvrtko Ursulin]

On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> crtc->base.primary->fb was used everywhere. Use fb to temporarily point
> there and don't forget to assign fb to its final destination at the end.
>
> v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
>   1 file changed, 27 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a84e47..6c042eb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6459,9 +6459,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
>   	int pipe = crtc->pipe, plane = crtc->plane;
>   	int fourcc, pixel_format;
>   	int aligned_height;
> +	struct drm_framebuffer *fb;
>
> -	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> -	if (!crtc->base.primary->fb) {
> +	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> +	if (!fb) {
>   		DRM_DEBUG_KMS("failed to alloc fb\n");
>   		return;

Is it of any consequence that after the change crtc->base.primary->fb 
will preserve it's content, while previously it would be NULLed on 
allocation failure? Probably not, just asking...

Tvrtko

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

Re: [PATCH 3/8] drm/i915: Unclutter the get_plane() functions

[Damien Lespiau]

On Thu, Oct 30, 2014 at 05:31:57PM +0000, Tvrtko Ursulin wrote:
> 
> On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> >crtc->base.primary->fb was used everywhere. Use fb to temporarily point
> >there and don't forget to assign fb to its final destination at the end.
> >
> >v2: Rebase on top of misc changes (mask of DSPSURF, PAGE_ALIGN)
> >
> >Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++--------------------
> >  1 file changed, 27 insertions(+), 34 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 2a84e47..6c042eb 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -6459,9 +6459,10 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
> >  	int pipe = crtc->pipe, plane = crtc->plane;
> >  	int fourcc, pixel_format;
> >  	int aligned_height;
> >+	struct drm_framebuffer *fb;
> >
> >-	crtc->base.primary->fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> >-	if (!crtc->base.primary->fb) {
> >+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> >+	if (!fb) {
> >  		DRM_DEBUG_KMS("failed to alloc fb\n");
> >  		return;
> 
> Is it of any consequence that after the change
> crtc->base.primary->fb will preserve it's content, while previously
> it would be NULLed on allocation failure? Probably not, just
> asking...

The get_plane_config() vfunc is only from intel_modeset_init() (to try
and inherit the BIOS fb) and so at the point, the crtc structure has
just been initialized and the initial get_pipe_config() doesn't touch
this fb field. crtc->base.primary->fb has not other choice than being
NULL at this point then I believe. So shouldn't matter.

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

Re: [PATCH 8/8] drm/i915/skl: Provide a Skylake version of get_plane_config()

[Tvrtko Ursulin]

On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> Universal planes have changed a bit the register organization.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 108 ++++++++++++++++++++++++++++++++---
>   1 file changed, 101 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 02b2a97..33e8112 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2314,6 +2314,32 @@ static int i9xx_format_to_fourcc(int format)
>   	}
>   }
>
> +static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
> +{
> +	switch (format) {
> +	case PLANE_CTL_FORMAT_RGB_565:
> +		return DRM_FORMAT_RGB565;
> +	default:
> +	case PLANE_CTL_FORMAT_XRGB_8888:
> +		if (rgb_order) {
> +			if (alpha)
> +				return DRM_FORMAT_ABGR8888;
> +			else
> +				return DRM_FORMAT_XBGR8888;
> +		} else {
> +			if (alpha)
> +				return DRM_FORMAT_ARGB8888;
> +			else
> +				return DRM_FORMAT_XRGB8888;
> +		}
> +	case PLANE_CTL_FORMAT_XRGB_2101010:
> +		if (rgb_order)
> +			return DRM_FORMAT_XBGR2101010;
> +		else
> +			return DRM_FORMAT_XRGB2101010;
> +	}
> +}

Are RGB and BGR wrong way round here?

>   static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
>   				  struct intel_plane_config *plane_config)
>   {
> @@ -7456,6 +7482,69 @@ static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
>   				     &pipe_config->fdi_m_n, NULL);
>   }
>
> +static void skylake_get_plane_config(struct intel_crtc *crtc,
> +				      struct intel_plane_config *plane_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 val, base, offset, stride_mult;
> +	int pipe = crtc->pipe;
> +	int fourcc, pixel_format;
> +	int aligned_height;
> +	struct drm_framebuffer *fb;
> +
> +	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> +	if (!fb) {
> +		DRM_DEBUG_KMS("failed to alloc fb\n");
> +		return;
> +	}
> +
> +	val = I915_READ(PLANE_CTL(pipe, 0));
> +	if (val & PLANE_CTL_TILED_MASK)
> +		plane_config->tiling = I915_TILING_X;
> +
> +	pixel_format = val & PLANE_CTL_FORMAT_MASK;
> +	fourcc = skl_format_to_fourcc(pixel_format,
> +				      val & PLANE_CTL_ORDER_RGBX,
> +				      val & PLANE_CTL_ALPHA_MASK);
> +	fb->pixel_format = fourcc;
> +	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> +
> +	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
> +	plane_config->base = base;
> +
> +	offset = I915_READ(PLANE_OFFSET(pipe, 0));
> +
> +	val = I915_READ(PIPESRC(pipe));
> +	fb->width = ((val >> 16) & 0xfff) + 1;
> +	fb->height = ((val >> 0) & 0xfff) + 1;

Thinking how you are reading out plane register everywhere else, I 
assume this means _PIPEASRC is guaranteed to be configured to the same 
size as _PLANE_SIZE_1_A?

> +	val = I915_READ(PLANE_STRIDE(pipe, 0));
> +	switch (plane_config->tiling) {
> +	case I915_TILING_NONE:
> +		stride_mult = 64;
> +		break;
> +	case I915_TILING_X:
> +		stride_mult = 512;
> +		break;
> +	default:
> +		BUG();

As some other people I also dislike BUGs very much. WARN and put no fb 
on screen instead?

Regards,

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

Re: [PATCH 0/8] Skylake primary plane read-out

[Daniel Vetter]

On Wed, Oct 29, 2014 at 05:22:39PM +0000, Damien Lespiau wrote:
> Skylake changed a few things here and there in the plane registers and we
> weren't correctly reading out the primary plane state when trying to re-use the
> BIOS stolen allocated fb (especially the stride was all wrong). Of course,
> weird artefacts ensued when loading the driver.
> 
> This series implement get_plane_config() for SKL, preceded  by a few clean-up
> patches. No artefacts when loading the driver anymore, at least on this end.
> 
> Damien Lespiau (8):
>   drm/i915: Change plane_config to store a tiling_mode
>   drm/i915: Use a common function for computing the fb height alignment
>   drm/i915: Unclutter the get_plane() functions
>   drm/i915: Don't use crtc->plane in ILK+ get_config()
>   drm/i915: Use pipe_name() in the get_plane_config() functions
>   drm/i915: Make intel_format_to_fourcc() static
>   drm/i915/skl: intel_format_to_fourcc() doesn't work for SKL planes
>   drm/i915/skl: Provide a Skylake version of get_plane_config()

Since you noodle around in this area: Feel like throwing an
s/plane_config/initial_plane_config/ at (v)funcs and structs in this area?

We're already growing a proper plane_config struct for atomic modesets,
and it's getting confusing ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/skl: Provide a Skylake version of get_plane_config()

[Damien Lespiau]

On Fri, Oct 31, 2014 at 11:38:34AM +0000, Tvrtko Ursulin wrote:
> 
> On 10/29/2014 05:22 PM, Damien Lespiau wrote:
> >Universal planes have changed a bit the register organization.
> >
> >Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_display.c | 108 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 101 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 02b2a97..33e8112 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -2314,6 +2314,32 @@ static int i9xx_format_to_fourcc(int format)
> >  	}
> >  }
> >
> >+static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
> >+{
> >+	switch (format) {
> >+	case PLANE_CTL_FORMAT_RGB_565:
> >+		return DRM_FORMAT_RGB565;
> >+	default:
> >+	case PLANE_CTL_FORMAT_XRGB_8888:
> >+		if (rgb_order) {
> >+			if (alpha)
> >+				return DRM_FORMAT_ABGR8888;
> >+			else
> >+				return DRM_FORMAT_XBGR8888;
> >+		} else {
> >+			if (alpha)
> >+				return DRM_FORMAT_ARGB8888;
> >+			else
> >+				return DRM_FORMAT_XRGB8888;
> >+		}
> >+	case PLANE_CTL_FORMAT_XRGB_2101010:
> >+		if (rgb_order)
> >+			return DRM_FORMAT_XBGR2101010;
> >+		else
> >+			return DRM_FORMAT_XRGB2101010;
> >+	}
> >+}
> 
> Are RGB and BGR wrong way round here?

Nop that's due to a mismatch of endianness between the DRM defines and
the register field (eg. intel_sprite.c is full of those inversions).

> >  static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
> >  				  struct intel_plane_config *plane_config)
> >  {
> >@@ -7456,6 +7482,69 @@ static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
> >  				     &pipe_config->fdi_m_n, NULL);
> >  }
> >
> >+static void skylake_get_plane_config(struct intel_crtc *crtc,
> >+				      struct intel_plane_config *plane_config)
> >+{
> >+	struct drm_device *dev = crtc->base.dev;
> >+	struct drm_i915_private *dev_priv = dev->dev_private;
> >+	u32 val, base, offset, stride_mult;
> >+	int pipe = crtc->pipe;
> >+	int fourcc, pixel_format;
> >+	int aligned_height;
> >+	struct drm_framebuffer *fb;
> >+
> >+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
> >+	if (!fb) {
> >+		DRM_DEBUG_KMS("failed to alloc fb\n");
> >+		return;
> >+	}
> >+
> >+	val = I915_READ(PLANE_CTL(pipe, 0));
> >+	if (val & PLANE_CTL_TILED_MASK)
> >+		plane_config->tiling = I915_TILING_X;
> >+
> >+	pixel_format = val & PLANE_CTL_FORMAT_MASK;
> >+	fourcc = skl_format_to_fourcc(pixel_format,
> >+				      val & PLANE_CTL_ORDER_RGBX,
> >+				      val & PLANE_CTL_ALPHA_MASK);
> >+	fb->pixel_format = fourcc;
> >+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> >+
> >+	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
> >+	plane_config->base = base;
> >+
> >+	offset = I915_READ(PLANE_OFFSET(pipe, 0));
> >+
> >+	val = I915_READ(PIPESRC(pipe));
> >+	fb->width = ((val >> 16) & 0xfff) + 1;
> >+	fb->height = ((val >> 0) & 0xfff) + 1;
> 
> Thinking how you are reading out plane register everywhere else, I assume
> this means _PIPEASRC is guaranteed to be configured to the same size as
> _PLANE_SIZE_1_A?

"guaranteed" is a strong word, but yes, I don't expect other
configuration. That said, we now have PLANE_SIZE on gen9+, so we might
as well use it, that's indeed more correct.

> >+	val = I915_READ(PLANE_STRIDE(pipe, 0));
> >+	switch (plane_config->tiling) {
> >+	case I915_TILING_NONE:
> >+		stride_mult = 64;
> >+		break;
> >+	case I915_TILING_X:
> >+		stride_mult = 512;
> >+		break;
> >+	default:
> >+		BUG();
> 
> As some other people I also dislike BUGs very much. WARN and put no fb on
> screen instead?

Sure, removed the BUG().

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

Re: [PATCH 8/8] drm/i915/skl: Provide a Skylake version of get_plane_config()

[Daniel Vetter]

On Mon, Jan 19, 2015 at 06:27:49PM +0000, Damien Lespiau wrote:
> On Fri, Oct 31, 2014 at 11:38:34AM +0000, Tvrtko Ursulin wrote:
> > >+	default:
> > >+		BUG();
> > 
> > As some other people I also dislike BUGs very much. WARN and put no fb on
> > screen instead?
> 
> Sure, removed the BUG().

MISSING_CASE(); is what you're looking for for this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8 v4] drm/i915/skl: Provide a Skylake version of get_plane_config()

[Damien Lespiau]

Universal planes have changed a bit the register organization.

v2: Rebase on top of the latest drm-intel-nightly

v3: Use PLANE_SIZE to retrieve the fb size (Tvrtko)
    Don't use BUG() (Tvrtko)

v4: Use MISSING_CASE (Daniel)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 114 ++++++++++++++++++++++++++++++++---
 1 file changed, 107 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 08a7130..0501252 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2337,6 +2337,32 @@ static int i9xx_format_to_fourcc(int format)
 	}
 }
 
+static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
+{
+	switch (format) {
+	case PLANE_CTL_FORMAT_RGB_565:
+		return DRM_FORMAT_RGB565;
+	default:
+	case PLANE_CTL_FORMAT_XRGB_8888:
+		if (rgb_order) {
+			if (alpha)
+				return DRM_FORMAT_ABGR8888;
+			else
+				return DRM_FORMAT_XBGR8888;
+		} else {
+			if (alpha)
+				return DRM_FORMAT_ARGB8888;
+			else
+				return DRM_FORMAT_XRGB8888;
+		}
+	case PLANE_CTL_FORMAT_XRGB_2101010:
+		if (rgb_order)
+			return DRM_FORMAT_XBGR2101010;
+		else
+			return DRM_FORMAT_XRGB2101010;
+	}
+}
+
 static bool intel_alloc_plane_obj(struct intel_crtc *crtc,
 				  struct intel_plane_config *plane_config)
 {
@@ -7565,6 +7591,74 @@ static void skylake_get_pfit_config(struct intel_crtc *crtc,
 	}
 }
 
+static void skylake_get_plane_config(struct intel_crtc *crtc,
+				      struct intel_plane_config *plane_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val, base, offset, stride_mult;
+	int pipe = crtc->pipe;
+	int fourcc, pixel_format;
+	int aligned_height;
+	struct drm_framebuffer *fb;
+
+	fb = kzalloc(sizeof(struct intel_framebuffer), GFP_KERNEL);
+	if (!fb) {
+		DRM_DEBUG_KMS("failed to alloc fb\n");
+		return;
+	}
+
+	val = I915_READ(PLANE_CTL(pipe, 0));
+	if (val & PLANE_CTL_TILED_MASK)
+		plane_config->tiling = I915_TILING_X;
+
+	pixel_format = val & PLANE_CTL_FORMAT_MASK;
+	fourcc = skl_format_to_fourcc(pixel_format,
+				      val & PLANE_CTL_ORDER_RGBX,
+				      val & PLANE_CTL_ALPHA_MASK);
+	fb->pixel_format = fourcc;
+	fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
+
+	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
+	plane_config->base = base;
+
+	offset = I915_READ(PLANE_OFFSET(pipe, 0));
+
+	val = I915_READ(PLANE_SIZE(pipe, 0));
+	fb->height = ((val >> 16) & 0xfff) + 1;
+	fb->width = ((val >> 0) & 0x1fff) + 1;
+
+	val = I915_READ(PLANE_STRIDE(pipe, 0));
+	switch (plane_config->tiling) {
+	case I915_TILING_NONE:
+		stride_mult = 64;
+		break;
+	case I915_TILING_X:
+		stride_mult = 512;
+		break;
+	default:
+		MISSING_CASE(plane_config->tiling);
+		goto error;
+	}
+	fb->pitches[0] = (val & 0x3ff) * stride_mult;
+
+	aligned_height = intel_fb_align_height(dev, fb->height,
+					       plane_config->tiling);
+
+	plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE);
+
+	DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      pipe_name(pipe), fb->width, fb->height,
+		      fb->bits_per_pixel, base, fb->pitches[0],
+		      plane_config->size);
+
+	crtc->base.primary->fb = fb;
+	return;
+
+error:
+	kfree(fb);
+}
+
 static void ironlake_get_pfit_config(struct intel_crtc *crtc,
 				     struct intel_crtc_config *pipe_config)
 {
@@ -12658,7 +12752,17 @@ static void intel_init_display(struct drm_device *dev)
 	else
 		dev_priv->display.find_dpll = i9xx_find_best_dpll;
 
-	if (HAS_DDI(dev)) {
+	if (INTEL_INFO(dev)->gen >= 9) {
+		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
+		dev_priv->display.get_plane_config = skylake_get_plane_config;
+		dev_priv->display.crtc_compute_clock =
+			haswell_crtc_compute_clock;
+		dev_priv->display.crtc_enable = haswell_crtc_enable;
+		dev_priv->display.crtc_disable = haswell_crtc_disable;
+		dev_priv->display.off = ironlake_crtc_off;
+		dev_priv->display.update_primary_plane =
+			skylake_update_primary_plane;
+	} else if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_plane_config = ironlake_get_plane_config;
 		dev_priv->display.crtc_compute_clock =
@@ -12666,12 +12770,8 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
 		dev_priv->display.off = ironlake_crtc_off;
-		if (INTEL_INFO(dev)->gen >= 9)
-			dev_priv->display.update_primary_plane =
-				skylake_update_primary_plane;
-		else
-			dev_priv->display.update_primary_plane =
-				ironlake_update_primary_plane;
+		dev_priv->display.update_primary_plane =
+			ironlake_update_primary_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.get_plane_config = ironlake_get_plane_config;
-- 
1.8.3.1

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