Re: [Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4

2013-11-26 Thread Chris Wilson 
On Mon, Nov 25, 2013 at 03:51:17PM -0800, Jesse Barnes wrote:
 Read out the current plane configuration at init time into a new
 plane_config structure.  This allows us to track any existing
 framebuffers attached to the plane and potentially re-use them in our
 fbdev code for a smooth handoff.
 
 v2: update for new pitch_for_width function (Jesse)
 comment how get_plane_config works with shared fbs (Jesse)
 v3: s/ARGB/XRGB (Ville)
 use pipesrc width/height (Ville)
 fix fourcc comment (Bob)
 use drm_format_plane_cpp (Ville)
 v4: use fb for tracking fb data object (Ville)
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---

 @@ -10879,6 +10992,18 @@ void intel_modeset_init(struct drm_device *dev)
  
   /* Just in case the BIOS is doing something questionable. */
   intel_disable_fbc(dev);
 +
 + intel_modeset_setup_hw_state(dev, false);
 +
 + list_for_each_entry(crtc, dev-mode_config.crtc_list,
 + base.head) {
 + if (!crtc-active)
 + continue;
 +
 + if (dev_priv-display.get_plane_config)
 + dev_priv-display.get_plane_config(crtc,
 +crtc-plane_config);

The trick is that here if we do not retreive the current config,
including the preallocated fb, we *must* disable the output. In this
case, it would be a step in intel_sanitize_crtc() to disable the CRTC if
it is enabled but we have no preserved fb.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4

2013-11-26 Thread Jesse Barnes 
On Tue, 26 Nov 2013 13:57:10 +
Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Mon, Nov 25, 2013 at 03:51:17PM -0800, Jesse Barnes wrote:
  Read out the current plane configuration at init time into a new
  plane_config structure.  This allows us to track any existing
  framebuffers attached to the plane and potentially re-use them in our
  fbdev code for a smooth handoff.
  
  v2: update for new pitch_for_width function (Jesse)
  comment how get_plane_config works with shared fbs (Jesse)
  v3: s/ARGB/XRGB (Ville)
  use pipesrc width/height (Ville)
  fix fourcc comment (Bob)
  use drm_format_plane_cpp (Ville)
  v4: use fb for tracking fb data object (Ville)
  
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
 
  @@ -10879,6 +10992,18 @@ void intel_modeset_init(struct drm_device *dev)
   
  /* Just in case the BIOS is doing something questionable. */
  intel_disable_fbc(dev);
  +
  +   intel_modeset_setup_hw_state(dev, false);
  +
  +   list_for_each_entry(crtc, dev-mode_config.crtc_list,
  +   base.head) {
  +   if (!crtc-active)
  +   continue;
  +
  +   if (dev_priv-display.get_plane_config)
  +   dev_priv-display.get_plane_config(crtc,
  +  crtc-plane_config);
 
 The trick is that here if we do not retreive the current config,
 including the preallocated fb, we *must* disable the output. In this
 case, it would be a step in intel_sanitize_crtc() to disable the CRTC if
 it is enabled but we have no preserved fb.

Yeah, but I thought since that's been broken for awhile, it should come
in as a separate bug fix.  I can do a patch on top of this if the rest
looks ok.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4

2013-11-26 Thread Daniel Vetter 
On Mon, Nov 25, 2013 at 03:51:17PM -0800, Jesse Barnes wrote:
 Read out the current plane configuration at init time into a new
 plane_config structure.  This allows us to track any existing
 framebuffers attached to the plane and potentially re-use them in our
 fbdev code for a smooth handoff.
 
 v2: update for new pitch_for_width function (Jesse)
 comment how get_plane_config works with shared fbs (Jesse)
 v3: s/ARGB/XRGB (Ville)
 use pipesrc width/height (Ville)
 fix fourcc comment (Bob)
 use drm_format_plane_cpp (Ville)
 v4: use fb for tracking fb data object (Ville)
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/i915_drv.h  |   3 +
  drivers/gpu/drm/i915/intel_display.c | 127 
 ++-
  drivers/gpu/drm/i915/intel_drv.h |   8 +++
  3 files changed, 136 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 14f250a..6569e93 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -365,6 +365,7 @@ struct drm_i915_error_state {
  
  struct intel_connector;
  struct intel_crtc_config;
 +struct intel_plane_config;
  struct intel_crtc;
  struct intel_limit;
  struct dpll;
 @@ -403,6 +404,8 @@ struct drm_i915_display_funcs {
* fills out the pipe-config with the hw state. */
   bool (*get_pipe_config)(struct intel_crtc *,
   struct intel_crtc_config *);
 + void (*get_plane_config)(struct intel_crtc *,
 +  struct intel_plane_config *);
   int (*crtc_mode_set)(struct drm_crtc *crtc,
int x, int y,
struct drm_framebuffer *old_fb);
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 321d751..089128b 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2002,6 +2002,27 @@ unsigned long intel_gen4_compute_page_offset(int *x, 
 int *y,
   }
  }
  
 +int intel_format_to_fourcc(int format)
 +{
 + switch (format) {
 + case DISPPLANE_8BPP:
 + return DRM_FORMAT_C8;
 + case DISPPLANE_BGRX555:
 + return DRM_FORMAT_XRGB1555;
 + case DISPPLANE_BGRX565:
 + return DRM_FORMAT_RGB565;
 + default:
 + case DISPPLANE_BGRX888:
 + return DRM_FORMAT_XRGB;
 + case DISPPLANE_RGBX888:
 + return DRM_FORMAT_XBGR;
 + case DISPPLANE_BGRX101010:
 + return DRM_FORMAT_XRGB2101010;
 + case DISPPLANE_RGBX101010:
 + return DRM_FORMAT_XBGR2101010;
 + }
 +}
 +
  static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer 
 *fb,
int x, int y)
  {
 @@ -5474,6 +5495,95 @@ intel_framebuffer_pitch_for_width(struct 
 drm_i915_private *dev_priv, int width,
   return ALIGN(pitch, align);
  }
  
 +static void i9xx_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;
 + struct drm_i915_gem_object *obj = NULL;
 + struct drm_mode_fb_cmd2 mode_cmd = { 0 };
 + u32 val, base, offset;
 + int pipe = crtc-pipe, plane = crtc-plane;
 + int fourcc, pixel_format;
 +
 + plane_config-fb = kzalloc(sizeof(*plane_config-fb), GFP_KERNEL);
 + if (!plane_config-fb) {
 + DRM_DEBUG_KMS(failed to alloc fb\n);
 + return;
 + }
 +
 + val = I915_READ(DSPCNTR(plane));
 +
 + if (INTEL_INFO(dev)-gen = 4)
 + if (val  DISPPLANE_TILED)
 + plane_config-tiled = true;
 +
 + pixel_format = val  DISPPLANE_PIXFORMAT_MASK;
 + fourcc = intel_format_to_fourcc(pixel_format);
 + plane_config-fb-base.pixel_format = fourcc;
 + plane_config-fb-base.bits_per_pixel =
 + drm_format_plane_cpp(fourcc, 0) * 8;
 +
 + if (INTEL_INFO(dev)-gen = 4) {
 + if (plane_config-tiled)
 + offset = I915_READ(DSPTILEOFF(plane));
 + else
 + offset = I915_READ(DSPLINOFF(plane));
 + base = I915_READ(DSPSURF(plane))  0xf000;
 + } else {
 + base = I915_READ(DSPADDR(plane));
 + }
 +
 + val = I915_READ(PIPESRC(pipe));
 + plane_config-fb-base.width = ((val  16)  0xfff) + 1;
 + plane_config-fb-base.height = ((val  0)  0xfff) + 1;
 +
 + plane_config-fb-base.pitches[0] =
 + intel_framebuffer_pitch_for_width(dev_priv,
 +   plane_config-fb-base.width,
 +   
 plane_config-fb-base.bits_per_pixel,
 +   plane_config-tiled);

Shouldn't we read out the stride from the respective hw register, too?

 +
 +

Re: [Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4

2013-11-26 Thread Jesse Barnes 
On Tue, 26 Nov 2013 18:43:53 +0100
Daniel Vetter dan...@ffwll.ch wrote:
  +   plane_config-fb-base.pitches[0] =
  +   intel_framebuffer_pitch_for_width(dev_priv,
  + plane_config-fb-base.width,
  + 
  plane_config-fb-base.bits_per_pixel,
  + plane_config-tiled);
 
 Shouldn't we read out the stride from the respective hw register, too?

Hm that's quite an idea.

 
  +
  +   plane_config-size = ALIGN(plane_config-fb-base.pitches[0] *
  +  plane_config-fb-base.height, PAGE_SIZE);
 
 If you bother with tiling I think you also need to bother with correct
 size alignement.

Yeah just fixed up the framebuffer size function per Ville's comments.
I should be able to just use that.

  @@ -10562,6 +10672,7 @@ static void intel_init_display(struct drm_device 
  *dev)
  dev_priv-display.update_plane = ironlake_update_plane;
 
 Sad Haswell is sad it seems.

Yeah for two reasons: I'm not testing HSW at all, and I'm still fairly
ignorant of HSW display.  Well that and I don't want to pile on more
work for this patchset which has already seen a bunch of churn...

  +   if (dev_priv-display.get_plane_config)
  +   dev_priv-display.get_plane_config(crtc,
  +  crtc-plane_config);
  +   }
 
 If we go with Chris' suggestion to disable the crtc if we can't get at a
 sane framebuffer for it then this would make much more sense in the
 hardware state readout code. Especially since always having framebuffers
 around would allow us to ditch a bit of special-casing code all over the
 place.

I don't think so; reading out and allocating an fb on every plane
config readout would be overkill.  We'd need to poke around in the
struct for cross checking, then free it somewhere.  Plus it won't
always be preallocated, and creating a duplicate fb when the object
still exists would fail.

This is actually another argument for simply duplicating a few fields
from the fb struct into the plane config struct.  That makes cross
checking and readout simple, and allows us to allocate if possible in
the BIOS functions.

But damnit, then I'd have to drop back to an earlier version of the
patch and lose some changes... ugg

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/5] drm/i915: retrieve current fb config into new plane_config structure at init v4

2013-11-25 Thread Jesse Barnes 
Read out the current plane configuration at init time into a new
plane_config structure.  This allows us to track any existing
framebuffers attached to the plane and potentially re-use them in our
fbdev code for a smooth handoff.

v2: update for new pitch_for_width function (Jesse)
comment how get_plane_config works with shared fbs (Jesse)
v3: s/ARGB/XRGB (Ville)
use pipesrc width/height (Ville)
fix fourcc comment (Bob)
use drm_format_plane_cpp (Ville)
v4: use fb for tracking fb data object (Ville)

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/i915_drv.h  |   3 +
 drivers/gpu/drm/i915/intel_display.c | 127 ++-
 drivers/gpu/drm/i915/intel_drv.h |   8 +++
 3 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 14f250a..6569e93 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -365,6 +365,7 @@ struct drm_i915_error_state {
 
 struct intel_connector;
 struct intel_crtc_config;
+struct intel_plane_config;
 struct intel_crtc;
 struct intel_limit;
 struct dpll;
@@ -403,6 +404,8 @@ struct drm_i915_display_funcs {
 * fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
struct intel_crtc_config *);
+   void (*get_plane_config)(struct intel_crtc *,
+struct intel_plane_config *);
int (*crtc_mode_set)(struct drm_crtc *crtc,
 int x, int y,
 struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 321d751..089128b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2002,6 +2002,27 @@ unsigned long intel_gen4_compute_page_offset(int *x, int 
*y,
}
 }
 
+int intel_format_to_fourcc(int format)
+{
+   switch (format) {
+   case DISPPLANE_8BPP:
+   return DRM_FORMAT_C8;
+   case DISPPLANE_BGRX555:
+   return DRM_FORMAT_XRGB1555;
+   case DISPPLANE_BGRX565:
+   return DRM_FORMAT_RGB565;
+   default:
+   case DISPPLANE_BGRX888:
+   return DRM_FORMAT_XRGB;
+   case DISPPLANE_RGBX888:
+   return DRM_FORMAT_XBGR;
+   case DISPPLANE_BGRX101010:
+   return DRM_FORMAT_XRGB2101010;
+   case DISPPLANE_RGBX101010:
+   return DRM_FORMAT_XBGR2101010;
+   }
+}
+
 static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 int x, int y)
 {
@@ -5474,6 +5495,95 @@ intel_framebuffer_pitch_for_width(struct 
drm_i915_private *dev_priv, int width,
return ALIGN(pitch, align);
 }
 
+static void i9xx_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;
+   struct drm_i915_gem_object *obj = NULL;
+   struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+   u32 val, base, offset;
+   int pipe = crtc-pipe, plane = crtc-plane;
+   int fourcc, pixel_format;
+
+   plane_config-fb = kzalloc(sizeof(*plane_config-fb), GFP_KERNEL);
+   if (!plane_config-fb) {
+   DRM_DEBUG_KMS(failed to alloc fb\n);
+   return;
+   }
+
+   val = I915_READ(DSPCNTR(plane));
+
+   if (INTEL_INFO(dev)-gen = 4)
+   if (val  DISPPLANE_TILED)
+   plane_config-tiled = true;
+
+   pixel_format = val  DISPPLANE_PIXFORMAT_MASK;
+   fourcc = intel_format_to_fourcc(pixel_format);
+   plane_config-fb-base.pixel_format = fourcc;
+   plane_config-fb-base.bits_per_pixel =
+   drm_format_plane_cpp(fourcc, 0) * 8;
+
+   if (INTEL_INFO(dev)-gen = 4) {
+   if (plane_config-tiled)
+   offset = I915_READ(DSPTILEOFF(plane));
+   else
+   offset = I915_READ(DSPLINOFF(plane));
+   base = I915_READ(DSPSURF(plane))  0xf000;
+   } else {
+   base = I915_READ(DSPADDR(plane));
+   }
+
+   val = I915_READ(PIPESRC(pipe));
+   plane_config-fb-base.width = ((val  16)  0xfff) + 1;
+   plane_config-fb-base.height = ((val  0)  0xfff) + 1;
+
+   plane_config-fb-base.pitches[0] =
+   intel_framebuffer_pitch_for_width(dev_priv,
+ plane_config-fb-base.width,
+ 
plane_config-fb-base.bits_per_pixel,
+ plane_config-tiled);
+
+   plane_config-size = ALIGN(plane_config-fb-base.pitches[0] *
+  plane_config-fb-base.height, PAGE_SIZE);
+