[PATCH 0/7] drm/i915: fbdev mode restoration improvements

[PATCH 0/7] drm/i915: fbdev mode restoration improvements

[ville.syrjala]

This series attempts to make sure we turn off any cursor and sprite
planes when we restore the fbdev mode.

Some of this stuff depends on the other series I just posted:
'[PATCH 0/6] drm/i915: Unfify some modeset sequences'

I also noticed some crap when looking at the code, so I slapped on a
few cleanups to the end

[PATCH 1/7] drm: Add fb_helper->restore_fbdev_mode hook

[ville.syrjala]

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

Drivers may need to turn off overlay planes, cursors, etc. when
restoring the fbdev mode. So allow drivers to provide their own
version of drm_fb_helper_restore_fbdev_mode() that can take care
of such details.

Initially just plug in drm_fb_helper_restore_fbdev_mode for all
drivers.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/ast/ast_fb.c              | 1 +
 drivers/gpu/drm/cirrus/cirrus_fbdev.c     | 1 +
 drivers/gpu/drm/drm_fb_cma_helper.c       | 1 +
 drivers/gpu/drm/drm_fb_helper.c           | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
 drivers/gpu/drm/gma500/framebuffer.c      | 1 +
 drivers/gpu/drm/i915/intel_fb.c           | 1 +
 drivers/gpu/drm/mgag200/mgag200_fb.c      | 1 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 1 +
 drivers/gpu/drm/omapdrm/omap_fbdev.c      | 1 +
 drivers/gpu/drm/qxl/qxl_fb.c              | 1 +
 drivers/gpu/drm/radeon/radeon_fb.c        | 1 +
 drivers/gpu/drm/udl/udl_fb.c              | 1 +
 include/drm/drm_fb_helper.h               | 1 +
 14 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 34931fe..2c1f469 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -254,6 +254,7 @@ static struct drm_fb_helper_funcs ast_fb_helper_funcs = {
 	.gamma_set = ast_fb_gamma_set,
 	.gamma_get = ast_fb_gamma_get,
 	.fb_probe = astfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void ast_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index e25afcc..ee48a21 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -253,6 +253,7 @@ static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
 	.gamma_set = cirrus_crtc_fb_gamma_set,
 	.gamma_get = cirrus_crtc_fb_gamma_get,
 	.fb_probe = cirrusfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int cirrus_fbdev_init(struct cirrus_device *cdev)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 0b5af7d..f011628 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -330,6 +330,7 @@ err_drm_gem_cma_free_object:
 
 static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
 	.fb_probe = drm_fbdev_cma_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 /**
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b78cbe7..02c70b2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -315,7 +315,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
-		ret = drm_fb_helper_restore_fbdev_mode(helper);
+		ret = helper->funcs->restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 68f0045..6ed4065 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -228,6 +228,7 @@ out:
 
 static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
 	.fb_probe =	exynos_drm_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int exynos_drm_fbdev_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 1534e22..8d7f9c0 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -567,6 +567,7 @@ static struct drm_fb_helper_funcs psb_fb_helper_funcs = {
 	.gamma_set = psbfb_gamma_set,
 	.gamma_get = psbfb_gamma_get,
 	.fb_probe = psbfb_probe,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static int psb_fbdev_destroy(struct drm_device *dev, struct psb_fbdev *fbdev)
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 0e19e57..a04481f 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -187,6 +187,7 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void intel_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 421beab..23b8de2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -239,6 +239,7 @@ static struct drm_fb_helper_funcs mga_fb_helper_funcs = {
 	.gamma_set = mga_crtc_fb_gamma_set,
 	.gamma_get = mga_crtc_fb_gamma_get,
 	.fb_probe = mgag200fb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int mgag200_fbdev_init(struct mga_device *mdev)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index b035317..2a280d8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -435,6 +435,7 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_set = nouveau_fbcon_gamma_set,
 	.gamma_get = nouveau_fbcon_gamma_get,
 	.fb_probe = nouveau_fbcon_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index b11ce60..ff3bc81 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -297,6 +297,7 @@ static struct drm_fb_helper_funcs omap_fb_helper_funcs = {
 	.gamma_set = omap_crtc_fb_gamma_set,
 	.gamma_get = omap_crtc_fb_gamma_get,
 	.fb_probe = omap_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static struct drm_fb_helper *get_fb(struct fb_info *fbi)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index b3c5127..b10db0c 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -525,6 +525,7 @@ static struct drm_fb_helper_funcs qxl_fb_helper_funcs = {
 	.gamma_get = qxl_crtc_fb_gamma_get,
 	*/
 	.fb_probe = qxl_fb_find_or_create_single,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int qxl_fbdev_init(struct qxl_device *qdev)
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index b174674..2b1b211 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -335,6 +335,7 @@ static struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
 	.gamma_set = radeon_crtc_fb_gamma_set,
 	.gamma_get = radeon_crtc_fb_gamma_get,
 	.fb_probe = radeonfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int radeon_fbdev_init(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index dc0c065..75a44fd 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -561,6 +561,7 @@ static struct drm_fb_helper_funcs udl_fb_helper_funcs = {
 	.gamma_set = udl_crtc_fb_gamma_set,
 	.gamma_get = udl_crtc_fb_gamma_get,
 	.fb_probe = udlfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void udl_fbdev_destroy(struct drm_device *dev,
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 8230b46..9f5b9be 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -72,6 +72,7 @@ struct drm_fb_helper_funcs {
 			       struct drm_fb_helper_crtc **crtcs,
 			       struct drm_display_mode **modes,
 			       bool *enabled, int width, int height);
+	bool (*restore_fbdev_mode)(struct drm_fb_helper *fb_helper);
 };
 
 struct drm_fb_helper_connector {
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/7] drm: Add fb_helper->restore_fbdev_mode hook

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday 08 May 2013 12:55:16 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drivers may need to turn off overlay planes, cursors, etc. when
> restoring the fbdev mode. So allow drivers to provide their own
> version of drm_fb_helper_restore_fbdev_mode() that can take care
> of such details.
> 
> Initially just plug in drm_fb_helper_restore_fbdev_mode for all
> drivers.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/ast/ast_fb.c              | 1 +
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c     | 1 +
>  drivers/gpu/drm/drm_fb_cma_helper.c       | 1 +
>  drivers/gpu/drm/drm_fb_helper.c           | 2 +-
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
>  drivers/gpu/drm/gma500/framebuffer.c      | 1 +
>  drivers/gpu/drm/i915/intel_fb.c           | 1 +
>  drivers/gpu/drm/mgag200/mgag200_fb.c      | 1 +
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 1 +
>  drivers/gpu/drm/omapdrm/omap_fbdev.c      | 1 +
>  drivers/gpu/drm/qxl/qxl_fb.c              | 1 +
>  drivers/gpu/drm/radeon/radeon_fb.c        | 1 +
>  drivers/gpu/drm/udl/udl_fb.c              | 1 +
>  include/drm/drm_fb_helper.h               | 1 +
>  14 files changed, 14 insertions(+), 1 deletion(-)

[snip]

> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 8230b46..9f5b9be 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -72,6 +72,7 @@ struct drm_fb_helper_funcs {
>  			       struct drm_fb_helper_crtc **crtcs,
>  			       struct drm_display_mode **modes,
>  			       bool *enabled, int width, int height);
> +	bool (*restore_fbdev_mode)(struct drm_fb_helper *fb_helper);

Could you please document this new function in the struct drm_fb_helper_funcs 
kerneldoc block ?

>  };
> 
>  struct drm_fb_helper_connector {
-- 
Regards,

Laurent Pinchart

[PATCH v2] drm: Add fb_helper->restore_fbdev_mode hook

[ville.syrjala]

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

Drivers may need to turn off overlay planes, cursors, etc. when
restoring the fbdev mode. So allow drivers to provide their own
version of drm_fb_helper_restore_fbdev_mode() that can take care
of such details.

Initially just plug in drm_fb_helper_restore_fbdev_mode for all
drivers.

v2: Add kernel-doc for the new hook

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/ast/ast_fb.c              | 1 +
 drivers/gpu/drm/cirrus/cirrus_fbdev.c     | 1 +
 drivers/gpu/drm/drm_fb_cma_helper.c       | 1 +
 drivers/gpu/drm/drm_fb_helper.c           | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
 drivers/gpu/drm/gma500/framebuffer.c      | 1 +
 drivers/gpu/drm/i915/intel_fb.c           | 1 +
 drivers/gpu/drm/mgag200/mgag200_fb.c      | 1 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 1 +
 drivers/gpu/drm/omapdrm/omap_fbdev.c      | 1 +
 drivers/gpu/drm/qxl/qxl_fb.c              | 1 +
 drivers/gpu/drm/radeon/radeon_fb.c        | 1 +
 drivers/gpu/drm/udl/udl_fb.c              | 1 +
 include/drm/drm_fb_helper.h               | 2 ++
 14 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 34931fe..2c1f469 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -254,6 +254,7 @@ static struct drm_fb_helper_funcs ast_fb_helper_funcs = {
 	.gamma_set = ast_fb_gamma_set,
 	.gamma_get = ast_fb_gamma_get,
 	.fb_probe = astfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void ast_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index e25afcc..ee48a21 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -253,6 +253,7 @@ static struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
 	.gamma_set = cirrus_crtc_fb_gamma_set,
 	.gamma_get = cirrus_crtc_fb_gamma_get,
 	.fb_probe = cirrusfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int cirrus_fbdev_init(struct cirrus_device *cdev)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 0b5af7d..f011628 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -330,6 +330,7 @@ err_drm_gem_cma_free_object:
 
 static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
 	.fb_probe = drm_fbdev_cma_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 /**
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b78cbe7..02c70b2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -315,7 +315,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
-		ret = drm_fb_helper_restore_fbdev_mode(helper);
+		ret = helper->funcs->restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 68f0045..6ed4065 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -228,6 +228,7 @@ out:
 
 static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
 	.fb_probe =	exynos_drm_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int exynos_drm_fbdev_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 1534e22..8d7f9c0 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -567,6 +567,7 @@ static struct drm_fb_helper_funcs psb_fb_helper_funcs = {
 	.gamma_set = psbfb_gamma_set,
 	.gamma_get = psbfb_gamma_get,
 	.fb_probe = psbfb_probe,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static int psb_fbdev_destroy(struct drm_device *dev, struct psb_fbdev *fbdev)
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 0e19e57..a04481f 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -187,6 +187,7 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void intel_fbdev_destroy(struct drm_device *dev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 421beab..23b8de2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -239,6 +239,7 @@ static struct drm_fb_helper_funcs mga_fb_helper_funcs = {
 	.gamma_set = mga_crtc_fb_gamma_set,
 	.gamma_get = mga_crtc_fb_gamma_get,
 	.fb_probe = mgag200fb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int mgag200_fbdev_init(struct mga_device *mdev)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index b035317..2a280d8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -435,6 +435,7 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_set = nouveau_fbcon_gamma_set,
 	.gamma_get = nouveau_fbcon_gamma_get,
 	.fb_probe = nouveau_fbcon_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index b11ce60..ff3bc81 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -297,6 +297,7 @@ static struct drm_fb_helper_funcs omap_fb_helper_funcs = {
 	.gamma_set = omap_crtc_fb_gamma_set,
 	.gamma_get = omap_crtc_fb_gamma_get,
 	.fb_probe = omap_fbdev_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static struct drm_fb_helper *get_fb(struct fb_info *fbi)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index b3c5127..b10db0c 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -525,6 +525,7 @@ static struct drm_fb_helper_funcs qxl_fb_helper_funcs = {
 	.gamma_get = qxl_crtc_fb_gamma_get,
 	*/
 	.fb_probe = qxl_fb_find_or_create_single,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int qxl_fbdev_init(struct qxl_device *qdev)
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index b174674..2b1b211 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -335,6 +335,7 @@ static struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
 	.gamma_set = radeon_crtc_fb_gamma_set,
 	.gamma_get = radeon_crtc_fb_gamma_get,
 	.fb_probe = radeonfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 int radeon_fbdev_init(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index dc0c065..75a44fd 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -561,6 +561,7 @@ static struct drm_fb_helper_funcs udl_fb_helper_funcs = {
 	.gamma_set = udl_crtc_fb_gamma_set,
 	.gamma_get = udl_crtc_fb_gamma_get,
 	.fb_probe = udlfb_create,
+	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
 };
 
 static void udl_fbdev_destroy(struct drm_device *dev,
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 471f276..146abb6 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -58,6 +58,7 @@ struct drm_fb_helper_surface_size {
  *            structure. Futhermore it also needs to allocate the drm
  *            framebuffer used to back the fbdev.
  * @initial_config: Setup an initial fbdev display configuration
+ * @restore_fbdev_mode: Restore the fbdev display configuration (eg. to show an oops)
  *
  * Driver callbacks used by the fbdev emulation helper library.
  */
@@ -73,6 +74,7 @@ struct drm_fb_helper_funcs {
 			       struct drm_fb_helper_crtc **crtcs,
 			       struct drm_display_mode **modes,
 			       bool *enabled, int width, int height);
+	bool (*restore_fbdev_mode)(struct drm_fb_helper *fb_helper);
 };
 
 struct drm_fb_helper_connector {
-- 
1.8.1.5

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

Re: [PATCH v2] drm: Add fb_helper->restore_fbdev_mode hook

[Ville Syrjälä]

On Wed, May 08, 2013 at 04:39:58PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drivers may need to turn off overlay planes, cursors, etc. when
> restoring the fbdev mode. So allow drivers to provide their own
> version of drm_fb_helper_restore_fbdev_mode() that can take care
> of such details.
> 
> Initially just plug in drm_fb_helper_restore_fbdev_mode for all
> drivers.
> 
> v2: Add kernel-doc for the new hook

And naturally I forgot to mention that this now depends on the
"[PATCH 0/3] drm: kernel-doc fixes" set I just posted.


-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2] drm: Add fb_helper->restore_fbdev_mode hook

[Laurent Pinchart]

On Wednesday 08 May 2013 16:39:58 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Drivers may need to turn off overlay planes, cursors, etc. when
> restoring the fbdev mode. So allow drivers to provide their own
> version of drm_fb_helper_restore_fbdev_mode() that can take care
> of such details.
> 
> Initially just plug in drm_fb_helper_restore_fbdev_mode for all
> drivers.
> 
> v2: Add kernel-doc for the new hook
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/ast/ast_fb.c              | 1 +
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c     | 1 +
>  drivers/gpu/drm/drm_fb_cma_helper.c       | 1 +
>  drivers/gpu/drm/drm_fb_helper.c           | 2 +-
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
>  drivers/gpu/drm/gma500/framebuffer.c      | 1 +
>  drivers/gpu/drm/i915/intel_fb.c           | 1 +
>  drivers/gpu/drm/mgag200/mgag200_fb.c      | 1 +
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c   | 1 +
>  drivers/gpu/drm/omapdrm/omap_fbdev.c      | 1 +
>  drivers/gpu/drm/qxl/qxl_fb.c              | 1 +
>  drivers/gpu/drm/radeon/radeon_fb.c        | 1 +
>  drivers/gpu/drm/udl/udl_fb.c              | 1 +
>  include/drm/drm_fb_helper.h               | 2 ++
>  14 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> index 34931fe..2c1f469 100644
> --- a/drivers/gpu/drm/ast/ast_fb.c
> +++ b/drivers/gpu/drm/ast/ast_fb.c
> @@ -254,6 +254,7 @@ static struct drm_fb_helper_funcs ast_fb_helper_funcs =
> { .gamma_set = ast_fb_gamma_set,
>  	.gamma_get = ast_fb_gamma_get,
>  	.fb_probe = astfb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  static void ast_fbdev_destroy(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index e25afcc..ee48a21 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -253,6 +253,7 @@ static struct drm_fb_helper_funcs cirrus_fb_helper_funcs
> = { .gamma_set = cirrus_crtc_fb_gamma_set,
>  	.gamma_get = cirrus_crtc_fb_gamma_get,
>  	.fb_probe = cirrusfb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  int cirrus_fbdev_init(struct cirrus_device *cdev)
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c index 0b5af7d..f011628 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -330,6 +330,7 @@ err_drm_gem_cma_free_object:
> 
>  static struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
>  	.fb_probe = drm_fbdev_cma_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  /**
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index b78cbe7..02c70b2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -315,7 +315,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
>  		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  			continue;
> 
> -		ret = drm_fb_helper_restore_fbdev_mode(helper);
> +		ret = helper->funcs->restore_fbdev_mode(helper);
>  		if (ret)
>  			error = true;
>  	}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 68f0045..6ed4065 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -228,6 +228,7 @@ out:
> 
>  static struct drm_fb_helper_funcs exynos_drm_fb_helper_funcs = {
>  	.fb_probe =	exynos_drm_fbdev_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  int exynos_drm_fbdev_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> b/drivers/gpu/drm/gma500/framebuffer.c index 1534e22..8d7f9c0 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -567,6 +567,7 @@ static struct drm_fb_helper_funcs psb_fb_helper_funcs =
> { .gamma_set = psbfb_gamma_set,
>  	.gamma_get = psbfb_gamma_get,
>  	.fb_probe = psbfb_probe,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  static int psb_fbdev_destroy(struct drm_device *dev, struct psb_fbdev
> *fbdev) diff --git a/drivers/gpu/drm/i915/intel_fb.c
> b/drivers/gpu/drm/i915/intel_fb.c index 0e19e57..a04481f 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -187,6 +187,7 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs
> = { .gamma_set = intel_crtc_fb_gamma_set,
>  	.gamma_get = intel_crtc_fb_gamma_get,
>  	.fb_probe = intelfb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  static void intel_fbdev_destroy(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c
> b/drivers/gpu/drm/mgag200/mgag200_fb.c index 421beab..23b8de2 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_fb.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
> @@ -239,6 +239,7 @@ static struct drm_fb_helper_funcs mga_fb_helper_funcs =
> { .gamma_set = mga_crtc_fb_gamma_set,
>  	.gamma_get = mga_crtc_fb_gamma_get,
>  	.fb_probe = mgag200fb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  int mgag200_fbdev_init(struct mga_device *mdev)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index b035317..2a280d8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -435,6 +435,7 @@ static struct drm_fb_helper_funcs
> nouveau_fbcon_helper_funcs = { .gamma_set = nouveau_fbcon_gamma_set,
>  	.gamma_get = nouveau_fbcon_gamma_get,
>  	.fb_probe = nouveau_fbcon_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> b/drivers/gpu/drm/omapdrm/omap_fbdev.c index b11ce60..ff3bc81 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -297,6 +297,7 @@ static struct drm_fb_helper_funcs omap_fb_helper_funcs =
> { .gamma_set = omap_crtc_fb_gamma_set,
>  	.gamma_get = omap_crtc_fb_gamma_get,
>  	.fb_probe = omap_fbdev_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  static struct drm_fb_helper *get_fb(struct fb_info *fbi)
> diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
> index b3c5127..b10db0c 100644
> --- a/drivers/gpu/drm/qxl/qxl_fb.c
> +++ b/drivers/gpu/drm/qxl/qxl_fb.c
> @@ -525,6 +525,7 @@ static struct drm_fb_helper_funcs qxl_fb_helper_funcs =
> { .gamma_get = qxl_crtc_fb_gamma_get,
>  	*/
>  	.fb_probe = qxl_fb_find_or_create_single,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  int qxl_fbdev_init(struct qxl_device *qdev)
> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c
> b/drivers/gpu/drm/radeon/radeon_fb.c index b174674..2b1b211 100644
> --- a/drivers/gpu/drm/radeon/radeon_fb.c
> +++ b/drivers/gpu/drm/radeon/radeon_fb.c
> @@ -335,6 +335,7 @@ static struct drm_fb_helper_funcs radeon_fb_helper_funcs
> = { .gamma_set = radeon_crtc_fb_gamma_set,
>  	.gamma_get = radeon_crtc_fb_gamma_get,
>  	.fb_probe = radeonfb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  int radeon_fbdev_init(struct radeon_device *rdev)
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index dc0c065..75a44fd 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -561,6 +561,7 @@ static struct drm_fb_helper_funcs udl_fb_helper_funcs =
> { .gamma_set = udl_crtc_fb_gamma_set,
>  	.gamma_get = udl_crtc_fb_gamma_get,
>  	.fb_probe = udlfb_create,
> +	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
>  };
> 
>  static void udl_fbdev_destroy(struct drm_device *dev,
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 471f276..146abb6 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -58,6 +58,7 @@ struct drm_fb_helper_surface_size {
>   *            structure. Futhermore it also needs to allocate the drm
>   *            framebuffer used to back the fbdev.
>   * @initial_config: Setup an initial fbdev display configuration
> + * @restore_fbdev_mode: Restore the fbdev display configuration (eg. to
> show an oops) *
>   * Driver callbacks used by the fbdev emulation helper library.
>   */
> @@ -73,6 +74,7 @@ struct drm_fb_helper_funcs {
>  			       struct drm_fb_helper_crtc **crtcs,
>  			       struct drm_display_mode **modes,
>  			       bool *enabled, int width, int height);
> +	bool (*restore_fbdev_mode)(struct drm_fb_helper *fb_helper);
>  };
> 
>  struct drm_fb_helper_connector {

-- 
Regards,

Laurent Pinchart

[PATCH 2/7] drm/i915: Don't enable cursors or sprites for fbdev

[ville.syrjala]

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

Check if the CRTC framebuffer matches the fbdev helper's framebuffer,
and if it does, doen't enable cursors/sprites.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 94d6604..cfe2803 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3264,12 +3264,23 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
 	}
 }
 
+static bool fbdev_active_on_crtc(const struct drm_crtc *crtc)
+{
+	const struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+
+	return dev_priv->fbdev && dev_priv->fbdev->helper.fb == crtc->fb;
+}
+
 static void intel_enable_planes(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	struct intel_plane *intel_plane;
 
+	/* don't enable sprite planes for fbdev */
+	if (fbdev_active_on_crtc(crtc))
+		return;
+
 	list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
 		if (intel_plane->pipe == pipe)
 			intel_plane_restore(&intel_plane->base);
@@ -6522,6 +6533,10 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	u32 base, pos;
 	bool visible;
 
+	/* don't enable cursors for fbdev */
+	if (on && fbdev_active_on_crtc(crtc))
+		return;
+
 	pos = 0;
 
 	if (on && crtc->enabled && crtc->fb) {
-- 
1.8.1.5

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

Re: [PATCH 2/7] drm/i915: Don't enable cursors or sprites for fbdev

[Daniel Vetter]

On Wed, May 08, 2013 at 12:55:17PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check if the CRTC framebuffer matches the fbdev helper's framebuffer,
> and if it does, doen't enable cursors/sprites.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This one here had conflicts. But it also feels redundant with patch 3,
since that one should make sure that we won't ever have a cursor/sprite
stuck around forever.

Of course it's a bit ugly since the cursor might flash shortly, but for
now I'll brush that off with our lack of atomic modesetting.

Or did I miss something subtly?
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 94d6604..cfe2803 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3264,12 +3264,23 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
>  	}
>  }
>  
> +static bool fbdev_active_on_crtc(const struct drm_crtc *crtc)
> +{
> +	const struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +
> +	return dev_priv->fbdev && dev_priv->fbdev->helper.fb == crtc->fb;
> +}
> +
>  static void intel_enable_planes(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  	struct intel_plane *intel_plane;
>  
> +	/* don't enable sprite planes for fbdev */
> +	if (fbdev_active_on_crtc(crtc))
> +		return;
> +
>  	list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
>  		if (intel_plane->pipe == pipe)
>  			intel_plane_restore(&intel_plane->base);
> @@ -6522,6 +6533,10 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>  	u32 base, pos;
>  	bool visible;
>  
> +	/* don't enable cursors for fbdev */
> +	if (on && fbdev_active_on_crtc(crtc))
> +		return;
> +
>  	pos = 0;
>  
>  	if (on && crtc->enabled && crtc->fb) {
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH 2/7] drm/i915: Don't enable cursors or sprites for fbdev

[Ville Syrjälä]

On Fri, May 24, 2013 at 11:20:21AM +0200, Daniel Vetter wrote:
> On Wed, May 08, 2013 at 12:55:17PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Check if the CRTC framebuffer matches the fbdev helper's framebuffer,
> > and if it does, doen't enable cursors/sprites.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This one here had conflicts. But it also feels redundant with patch 3,
> since that one should make sure that we won't ever have a cursor/sprite
> stuck around forever.
> 
> Of course it's a bit ugly since the cursor might flash shortly, but for
> now I'll brush that off with our lack of atomic modesetting.
> 
> Or did I miss something subtly?

My initial idea was to avoid the flashing.

However, now that I think about it a bit more, we do call set_config
from the fb_helper set_par/pan_display hooks, which could then end up
enabling sprites/cursors w/ fbdev. So either we need to keep this patch,
or maybe provide custom set_par/pan_display to turn off
sprites/cursors as well.



> -Daniel
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 94d6604..cfe2803 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3264,12 +3264,23 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
> >  	}
> >  }
> >  
> > +static bool fbdev_active_on_crtc(const struct drm_crtc *crtc)
> > +{
> > +	const struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > +
> > +	return dev_priv->fbdev && dev_priv->fbdev->helper.fb == crtc->fb;
> > +}
> > +
> >  static void intel_enable_planes(struct drm_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> >  	struct intel_plane *intel_plane;
> >  
> > +	/* don't enable sprite planes for fbdev */
> > +	if (fbdev_active_on_crtc(crtc))
> > +		return;
> > +
> >  	list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
> >  		if (intel_plane->pipe == pipe)
> >  			intel_plane_restore(&intel_plane->base);
> > @@ -6522,6 +6533,10 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> >  	u32 base, pos;
> >  	bool visible;
> >  
> > +	/* don't enable cursors for fbdev */
> > +	if (on && fbdev_active_on_crtc(crtc))
> > +		return;
> > +
> >  	pos = 0;
> >  
> >  	if (on && crtc->enabled && crtc->fb) {
> > -- 
> > 1.8.1.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

[PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[ville.syrjala]

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

plane->enabled is never set, so this code didn't do anything.

If we end up doing a full modeset, sprites already get disabled. However
if we end up doing a simple set_base, we still need to turn off the sprites
manually.

And do the same for cursors, since we only want to see the primary plane
for fbdev.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cfe2803..cef0bff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9928,3 +9928,17 @@ intel_display_print_error_state(struct seq_file *m,
 	}
 }
 #endif
+
+void intel_disable_cursors_and_sprites(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
+		intel_crtc_update_cursor(crtc, false);
+	}
+
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+		intel_plane_disable(plane);
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cd1297e..a4e866a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -789,5 +789,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
 extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
 						 enum transcoder pch_transcoder,
 						 bool enable);
+extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index a04481f..57a082c 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -281,8 +281,6 @@ void intel_fb_restore_mode(struct drm_device *dev)
 {
 	int ret;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_plane *plane;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
 		return;
@@ -293,10 +291,8 @@ void intel_fb_restore_mode(struct drm_device *dev)
 	if (ret)
 		DRM_DEBUG("failed to restore crtc mode\n");
 
-	/* Be sure to shut off any planes that may be active */
-	list_for_each_entry(plane, &config->plane_list, head)
-		if (plane->enabled)
-			plane->funcs->disable_plane(plane);
+	/* in case we ended up doing just set_base above */
+	intel_disable_cursors_and_sprites(dev);
 
 	drm_modeset_unlock_all(dev);
 }
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Wed, May 08, 2013 at 12:55:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> If we end up doing a full modeset, sprites already get disabled. However
> if we end up doing a simple set_base, we still need to turn off the sprites
> manually.
> 
> And do the same for cursors, since we only want to see the primary plane
> for fbdev.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  8 ++------
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfe2803..cef0bff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9928,3 +9928,17 @@ intel_display_print_error_state(struct seq_file *m,
>  	}
>  }
>  #endif
> +
> +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> +		intel_crtc_update_cursor(crtc, false);
> +	}
> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +		intel_plane_disable(plane);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd1297e..a4e866a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -789,5 +789,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>  						 enum transcoder pch_transcoder,
>  						 bool enable);
> +extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index a04481f..57a082c 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -281,8 +281,6 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  {
>  	int ret;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_plane *plane;
>  
>  	if (INTEL_INFO(dev)->num_pipes == 0)
>  		return;
> @@ -293,10 +291,8 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  	if (ret)
>  		DRM_DEBUG("failed to restore crtc mode\n");
>  
> -	/* Be sure to shut off any planes that may be active */
> -	list_for_each_entry(plane, &config->plane_list, head)
> -		if (plane->enabled)
> -			plane->funcs->disable_plane(plane);
> +	/* in case we ended up doing just set_base above */
> +	intel_disable_cursors_and_sprites(dev);

Since I did not merge patch 2 I've killed this comment here.
-Daniel
>  
>  	drm_modeset_unlock_all(dev);
>  }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Wed, May 08, 2013 at 12:55:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> plane->enabled is never set, so this code didn't do anything.
> 
> If we end up doing a full modeset, sprites already get disabled. However
> if we end up doing a simple set_base, we still need to turn off the sprites
> manually.
> 
> And do the same for cursors, since we only want to see the primary plane
> for fbdev.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_fb.c      |  8 ++------
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfe2803..cef0bff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9928,3 +9928,17 @@ intel_display_print_error_state(struct seq_file *m,
>  	}
>  }
>  #endif
> +
> +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> +		intel_crtc_update_cursor(crtc, false);
> +	}
> +
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +		intel_plane_disable(plane);

Somehow my tree doesn't have an intel_plane_disable and while hunting that
one down I've started to think a bit more ...

Why exactly do we need special driver callbacks for all this? Strictly
speaking fbcon is just another kms client, if it needs to dig around in
driver details that just means our interfaces are not good enough. I know
that there's some historical baggage left around, but for disabling
cursors and planes I don't see any need at all:

Can't we just loop over all crtcs and disable every cursor and then loop
over all sprites and also shut them all down when restoring the fbdev
mode? All without any new driver callbacks?

It might flicker like mad since we can't do it atomically, but then again
this would give us a neat in-kernel user for the atomic modeset stuff
right away ...

Cheers, Daniel

> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cd1297e..a4e866a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -789,5 +789,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>  						 enum transcoder pch_transcoder,
>  						 bool enable);
> +extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index a04481f..57a082c 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -281,8 +281,6 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  {
>  	int ret;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_plane *plane;
>  
>  	if (INTEL_INFO(dev)->num_pipes == 0)
>  		return;
> @@ -293,10 +291,8 @@ void intel_fb_restore_mode(struct drm_device *dev)
>  	if (ret)
>  		DRM_DEBUG("failed to restore crtc mode\n");
>  
> -	/* Be sure to shut off any planes that may be active */
> -	list_for_each_entry(plane, &config->plane_list, head)
> -		if (plane->enabled)
> -			plane->funcs->disable_plane(plane);
> +	/* in case we ended up doing just set_base above */
> +	intel_disable_cursors_and_sprites(dev);
>  
>  	drm_modeset_unlock_all(dev);
>  }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Ville Syrjälä]

On Fri, May 24, 2013 at 11:35:07AM +0200, Daniel Vetter wrote:
> On Wed, May 08, 2013 at 12:55:18PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > plane->enabled is never set, so this code didn't do anything.
> > 
> > If we end up doing a full modeset, sprites already get disabled. However
> > if we end up doing a simple set_base, we still need to turn off the sprites
> > manually.
> > 
> > And do the same for cursors, since we only want to see the primary plane
> > for fbdev.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_fb.c      |  8 ++------
> >  3 files changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index cfe2803..cef0bff 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9928,3 +9928,17 @@ intel_display_print_error_state(struct seq_file *m,
> >  	}
> >  }
> >  #endif
> > +
> > +void intel_disable_cursors_and_sprites(struct drm_device *dev)
> > +{
> > +	struct drm_crtc *crtc;
> > +	struct drm_plane *plane;
> > +
> > +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +		intel_crtc_dpms_overlay(to_intel_crtc(crtc), false);
> > +		intel_crtc_update_cursor(crtc, false);
> > +	}
> > +
> > +	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> > +		intel_plane_disable(plane);
> 
> Somehow my tree doesn't have an intel_plane_disable and while hunting that
> one down I've started to think a bit more ...

Yeah, that sucker was in the modeset sequence patch set. I should've
pulled it out into a separate patch.

> Why exactly do we need special driver callbacks for all this? Strictly
> speaking fbcon is just another kms client, if it needs to dig around in
> driver details that just means our interfaces are not good enough. I know
> that there's some historical baggage left around, but for disabling
> cursors and planes I don't see any need at all:
> 
> Can't we just loop over all crtcs and disable every cursor and then loop
> over all sprites and also shut them all down when restoring the fbdev
> mode? All without any new driver callbacks?

Some driver might want to use hardware cursor for fb_cursor, also some
drivers want to use planes for fbdev scanout, which as you know is
something I want for i915 as well in the future.

Also there's the whole state mismanagement. We don't track enough of
the state in the core to restore stuff properly, so if we disable
stuff, we can't re-enable it when switching back to the client using
sprites/planes. We could move the plane coordinate tracking to the
core easily enough (I even had a patch for that in some old atomic
branch). I haven't looked at out current cursor stuff closely enough
to figure out if we can do the same there, but currently the cursor
state is tracked purely inside the driver.

So for the short term I think it's easier to go with the new hook,
and let each driver manage their planes/cursors.

> 
> It might flicker like mad since we can't do it atomically, but then again
> this would give us a neat in-kernel user for the atomic modeset stuff
> right away ...

When we get to the atomic stuff, I'd like to keep a full state per
client, and then it'd be very easy to just blast the new state in when
swithing between clients.

> 
> Cheers, Daniel
> 
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index cd1297e..a4e866a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -789,5 +789,6 @@ extern bool intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
> >  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
> >  						 enum transcoder pch_transcoder,
> >  						 bool enable);
> > +extern void intel_disable_cursors_and_sprites(struct drm_device *dev);
> >  
> >  #endif /* __INTEL_DRV_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> > index a04481f..57a082c 100644
> > --- a/drivers/gpu/drm/i915/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/intel_fb.c
> > @@ -281,8 +281,6 @@ void intel_fb_restore_mode(struct drm_device *dev)
> >  {
> >  	int ret;
> >  	drm_i915_private_t *dev_priv = dev->dev_private;
> > -	struct drm_mode_config *config = &dev->mode_config;
> > -	struct drm_plane *plane;
> >  
> >  	if (INTEL_INFO(dev)->num_pipes == 0)
> >  		return;
> > @@ -293,10 +291,8 @@ void intel_fb_restore_mode(struct drm_device *dev)
> >  	if (ret)
> >  		DRM_DEBUG("failed to restore crtc mode\n");
> >  
> > -	/* Be sure to shut off any planes that may be active */
> > -	list_for_each_entry(plane, &config->plane_list, head)
> > -		if (plane->enabled)
> > -			plane->funcs->disable_plane(plane);
> > +	/* in case we ended up doing just set_base above */
> > +	intel_disable_cursors_and_sprites(dev);
> >  
> >  	drm_modeset_unlock_all(dev);
> >  }
> > -- 
> > 1.8.1.5
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Fri, May 24, 2013 at 11:59 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
>> Why exactly do we need special driver callbacks for all this? Strictly
>> speaking fbcon is just another kms client, if it needs to dig around in
>> driver details that just means our interfaces are not good enough. I know
>> that there's some historical baggage left around, but for disabling
>> cursors and planes I don't see any need at all:
>>
>> Can't we just loop over all crtcs and disable every cursor and then loop
>> over all sprites and also shut them all down when restoring the fbdev
>> mode? All without any new driver callbacks?
>
> Some driver might want to use hardware cursor for fb_cursor, also some
> drivers want to use planes for fbdev scanout, which as you know is
> something I want for i915 as well in the future.

I guess the aim is to use the plane scaler and blast the fbcon output
over whatever resolution we're currently displaying for e.g. an oops?
Even then I don't see how this is any different from Wayland blasting
a fullscreen app all over the screen. And if our drm core -> driver
interface isn't good enough for fbcon to do that, Wayland won't ever
get it right.

So all I'm saying is that proper fbcon safe/restore looks like a neat
in-kernel test-bed to check interfaces. And we should go with that
approach instead of rolling driver hacks. And splattering
is_fbcon_crtc checks all over _is_ imo a great hack.

> Also there's the whole state mismanagement. We don't track enough of
> the state in the core to restore stuff properly, so if we disable
> stuff, we can't re-enable it when switching back to the client using
> sprites/planes. We could move the plane coordinate tracking to the
> core easily enough (I even had a patch for that in some old atomic
> branch). I haven't looked at out current cursor stuff closely enough
> to figure out if we can do the same there, but currently the cursor
> state is tracked purely inside the driver.

Atm kms client state management is purely driven by vt switching, and
we don't have any guarantees at all that state survives a vt switch.
Hence both X and Wayland already fully restore kms state. So in the
current model we don't need any state safe/restore in the kernel.

Also I'm not sure whether doing that state safe/restore in the kernel
is the right approach to handle switching between multiple
compositors. I expect that we want a system compositor (for nice
transitions) and then probably also we need a new ownership model to
ensure that compositors don't have access to each anothers stuff.

> So for the short term I think it's easier to go with the new hook,
> and let each driver manage their planes/cursors.

Since we can't avoid the flickering right now (at least in the
set_base fastpath, which should be common) the only thing we need is
to loop over all cursors and sprites in fbdev_restore_mode and kill
them. X/Wayland will restore already. That's one small patch afaics
and so imo the right solution short-term.

And long-term we'd have a simple testcase for the atomic modeset
interfaces, which might help a bit with the dri-devel bikeshedding.

>> It might flicker like mad since we can't do it atomically, but then again
>> this would give us a neat in-kernel user for the atomic modeset stuff
>> right away ...
>
> When we get to the atomic stuff, I'd like to keep a full state per
> client, and then it'd be very easy to just blast the new state in when
> swithing between clients.

See above, I don't think we need that. Or at least there's a lot of
open questions around it imo.

Generally though I really think we should move away from driver
interfaces for fbcon. Currently we have
- fb allocation - I think that could be done with the dumb ioctl
interface instead, or at least it should be possible to do so. Tbh
haven't looked into it.
- initial mode selection. Can imo only be fixed for real with the
check-only mode of the atomic modeset. Till then we have a bit an
abstraction leak.
- gamma settings. Pure duplication with the kms interfaces, is on my
list of things to kill.

The _only_ places fbdev support should need driver hooks are imo are
if a driver wants extended fbdev emulation (e.g. render accel), and
for those cases it can simply overwrite the fbdev vtable with its own
functions (which might or might not call down into the fbdev helper).

But for basic modeset operations there's imo really no need for
special fbdev helper interfaces, our core drm -> driver interface
should be good enough.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Ville Syrjälä]

On Fri, May 24, 2013 at 05:14:40PM +0200, Daniel Vetter wrote:
> On Fri, May 24, 2013 at 11:59 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> >> Why exactly do we need special driver callbacks for all this? Strictly
> >> speaking fbcon is just another kms client, if it needs to dig around in
> >> driver details that just means our interfaces are not good enough. I know
> >> that there's some historical baggage left around, but for disabling
> >> cursors and planes I don't see any need at all:
> >>
> >> Can't we just loop over all crtcs and disable every cursor and then loop
> >> over all sprites and also shut them all down when restoring the fbdev
> >> mode? All without any new driver callbacks?
> >
> > Some driver might want to use hardware cursor for fb_cursor, also some
> > drivers want to use planes for fbdev scanout, which as you know is
> > something I want for i915 as well in the future.
> 
> I guess the aim is to use the plane scaler and blast the fbcon output
> over whatever resolution we're currently displaying for e.g. an oops?

I was mainly thinking about the current drivers that already depend on
planes for scanout. We don't want to go blindly disable all planes in
the core since the driver is the one that knows which plane it needs for
the crtc scanout duties.

> Even then I don't see how this is any different from Wayland blasting
> a fullscreen app all over the screen. And if our drm core -> driver
> interface isn't good enough for fbcon to do that, Wayland won't ever
> get it right.
> 
> So all I'm saying is that proper fbcon safe/restore looks like a neat
> in-kernel test-bed to check interfaces. And we should go with that
> approach instead of rolling driver hacks. And splattering
> is_fbcon_crtc checks all over _is_ imo a great hack.

Sure, it's a hack.

> 
> > Also there's the whole state mismanagement. We don't track enough of
> > the state in the core to restore stuff properly, so if we disable
> > stuff, we can't re-enable it when switching back to the client using
> > sprites/planes. We could move the plane coordinate tracking to the
> > core easily enough (I even had a patch for that in some old atomic
> > branch). I haven't looked at out current cursor stuff closely enough
> > to figure out if we can do the same there, but currently the cursor
> > state is tracked purely inside the driver.
> 
> Atm kms client state management is purely driven by vt switching, and
> we don't have any guarantees at all that state survives a vt switch.
> Hence both X and Wayland already fully restore kms state. So in the
> current model we don't need any state safe/restore in the kernel.
> 
> Also I'm not sure whether doing that state safe/restore in the kernel
> is the right approach to handle switching between multiple
> compositors. I expect that we want a system compositor (for nice
> transitions) and then probably also we need a new ownership model to
> ensure that compositors don't have access to each anothers stuff.

Good point.

> 
> > So for the short term I think it's easier to go with the new hook,
> > and let each driver manage their planes/cursors.
> 
> Since we can't avoid the flickering right now (at least in the
> set_base fastpath, which should be common) the only thing we need is
> to loop over all cursors and sprites in fbdev_restore_mode and kill
> them. X/Wayland will restore already. That's one small patch afaics
> and so imo the right solution short-term.

So what about set_par/pan_display? Do you want the fbdev restore thingy
to permanently disable the planes/cursors, as in update our s/w state to
indicate that they're disabled and hence won't get restored unless
someone explicitly re-enables them? Because that's not what the current
code does.

> 
> And long-term we'd have a simple testcase for the atomic modeset
> interfaces, which might help a bit with the dri-devel bikeshedding.
> 
> >> It might flicker like mad since we can't do it atomically, but then again
> >> this would give us a neat in-kernel user for the atomic modeset stuff
> >> right away ...
> >
> > When we get to the atomic stuff, I'd like to keep a full state per
> > client, and then it'd be very easy to just blast the new state in when
> > swithing between clients.
> 
> See above, I don't think we need that. Or at least there's a lot of
> open questions around it imo.
> 
> Generally though I really think we should move away from driver
> interfaces for fbcon. Currently we have
> - fb allocation - I think that could be done with the dumb ioctl
> interface instead, or at least it should be possible to do so. Tbh
> haven't looked into it.
> - initial mode selection. Can imo only be fixed for real with the
> check-only mode of the atomic modeset. Till then we have a bit an
> abstraction leak.
> - gamma settings. Pure duplication with the kms interfaces, is on my
> list of things to kill.
> 
> The _only_ places fbdev support should need driver hooks are imo are
> if a driver wants extended fbdev emulation (e.g. render accel), and
> for those cases it can simply overwrite the fbdev vtable with its own
> functions (which might or might not call down into the fbdev helper).
> 
> But for basic modeset operations there's imo really no need for
> special fbdev helper interfaces, our core drm -> driver interface
> should be good enough.
> 
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/7] drm/i915: Fix fbdev sprite disable code

[Daniel Vetter]

On Fri, May 24, 2013 at 7:06 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, May 24, 2013 at 05:14:40PM +0200, Daniel Vetter wrote:
>> On Fri, May 24, 2013 at 11:59 AM, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>> >
>> >> Why exactly do we need special driver callbacks for all this? Strictly
>> >> speaking fbcon is just another kms client, if it needs to dig around in
>> >> driver details that just means our interfaces are not good enough. I know
>> >> that there's some historical baggage left around, but for disabling
>> >> cursors and planes I don't see any need at all:
>> >>
>> >> Can't we just loop over all crtcs and disable every cursor and then loop
>> >> over all sprites and also shut them all down when restoring the fbdev
>> >> mode? All without any new driver callbacks?
>> >
>> > Some driver might want to use hardware cursor for fb_cursor, also some
>> > drivers want to use planes for fbdev scanout, which as you know is
>> > something I want for i915 as well in the future.
>>
>> I guess the aim is to use the plane scaler and blast the fbcon output
>> over whatever resolution we're currently displaying for e.g. an oops?
>
> I was mainly thinking about the current drivers that already depend on
> planes for scanout. We don't want to go blindly disable all planes in
> the core since the driver is the one that knows which plane it needs for
> the crtc scanout duties.

I guess those currently have a plane reserved for each crtc (or
otherwise need to be careful not to glober crtc state if the implicit
crtc plane is used as a real kms plane). I guess we want to eventually
make that implicit plane explicit, but I guess that can only be done
with a feature flag. At least I don't see any way to do this without
breaking the established kms abi.

Maybe Rob could chime in?

[snip]

>> > So for the short term I think it's easier to go with the new hook,
>> > and let each driver manage their planes/cursors.
>>
>> Since we can't avoid the flickering right now (at least in the
>> set_base fastpath, which should be common) the only thing we need is
>> to loop over all cursors and sprites in fbdev_restore_mode and kill
>> them. X/Wayland will restore already. That's one small patch afaics
>> and so imo the right solution short-term.
>
> So what about set_par/pan_display? Do you want the fbdev restore thingy
> to permanently disable the planes/cursors, as in update our s/w state to
> indicate that they're disabled and hence won't get restored unless
> someone explicitly re-enables them? Because that's not what the current
> code does.

Yeah, I think it should just clobber the kms state. Clients will
restore everything when vt-switching anyway and if we oops it doesn't
matter that much since the system is going down the toilet anyway.

That leaves kgdb, but since kgdb is currently broken when the active
client uses a sprite (which might cover up kgdb) I don't think it's
worse if it's broken in a different fashion afterwards (by killing the
sprite state once the client will be restored. And we should be able
to fix this by storing the sprite state somewhere ...

Maybe we need to add a better getfoo ioctl once we have atomic modeset
to better support a system compositor. That way it could reconstruct
the kms state and flip clients in userspace.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/7] drm/i915: Use a custom restore_fbdev_mode hook

[ville.syrjala]

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

Disable sprite planes and cursors when restoring the fbdev mode.
This is actually only needed in case the modeset gets optimized into a
set_base, but that's a fairly important case these days.

Should makes oopses more readable if they're not covered by spriteas and
cursors.

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

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 57a082c..a9e9fc0 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -183,11 +183,26 @@ out:
 	return ret;
 }
 
+static bool intel_fb_restore_fbdev_mode(struct drm_fb_helper *helper)
+{
+	struct drm_device *dev = helper->dev;
+	bool ret;
+
+	ret = drm_fb_helper_restore_fbdev_mode(helper);
+	if (ret)
+		DRM_DEBUG("failed to restore crtc mode\n");
+
+	/* in case we ended up doing just set_base above */
+	intel_disable_cursors_and_sprites(dev);
+
+	return ret;
+}
+
 static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 	.gamma_set = intel_crtc_fb_gamma_set,
 	.gamma_get = intel_crtc_fb_gamma_get,
 	.fb_probe = intelfb_create,
-	.restore_fbdev_mode = drm_fb_helper_restore_fbdev_mode,
+	.restore_fbdev_mode = intel_fb_restore_fbdev_mode,
 };
 
 static void intel_fbdev_destroy(struct drm_device *dev,
@@ -279,7 +294,6 @@ void intel_fb_output_poll_changed(struct drm_device *dev)
 
 void intel_fb_restore_mode(struct drm_device *dev)
 {
-	int ret;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
@@ -287,12 +301,7 @@ void intel_fb_restore_mode(struct drm_device *dev)
 
 	drm_modeset_lock_all(dev);
 
-	ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
-	if (ret)
-		DRM_DEBUG("failed to restore crtc mode\n");
-
-	/* in case we ended up doing just set_base above */
-	intel_disable_cursors_and_sprites(dev);
+	intel_fb_restore_fbdev_mode(&dev_priv->fbdev->helper);
 
 	drm_modeset_unlock_all(dev);
 }
-- 
1.8.1.5

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

[PATCH 5/7] drm/i915: Use container_of() in the fbdev code

[ville.syrjala]

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

Use container_of() instead of a cast to get struct intel_fbdev
from struct drm_fb_helper.

Also populate the fb_info->par correctly with the drm_fb_helper pointer
instead of the intel_fbdev pointer.

There's no actual functional change since the drm_fb_helper happens to
be the first member inside intel_fbdev.

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

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index a9e9fc0..1728cd9 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -60,8 +60,9 @@ static struct fb_ops intelfb_ops = {
 static int intelfb_create(struct drm_fb_helper *helper,
 			  struct drm_fb_helper_surface_size *sizes)
 {
-	struct intel_fbdev *ifbdev = (struct intel_fbdev *)helper;
-	struct drm_device *dev = ifbdev->helper.dev;
+	struct intel_fbdev *ifbdev =
+		container_of(helper, struct intel_fbdev, helper);
+	struct drm_device *dev = helper->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct fb_info *info;
 	struct drm_framebuffer *fb;
@@ -108,7 +109,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 		goto out_unpin;
 	}
 
-	info->par = ifbdev;
+	info->par = helper;
 
 	ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj);
 	if (ret)
-- 
1.8.1.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/7] drm/i915: s/drm_i915_private_t/struct drm_i915_private/

[ville.syrjala]

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

People don't like typedefs these days. Eliminate their use from intel_fb.c.

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

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 1728cd9..53ab710 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -234,7 +234,7 @@ static void intel_fbdev_destroy(struct drm_device *dev,
 int intel_fbdev_init(struct drm_device *dev)
 {
 	struct intel_fbdev *ifbdev;
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
 	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
@@ -259,7 +259,7 @@ int intel_fbdev_init(struct drm_device *dev)
 
 void intel_fbdev_initial_config(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
 	drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
@@ -267,7 +267,7 @@ void intel_fbdev_initial_config(struct drm_device *dev)
 
 void intel_fbdev_fini(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	if (!dev_priv->fbdev)
 		return;
 
@@ -278,7 +278,7 @@ void intel_fbdev_fini(struct drm_device *dev)
 
 void intel_fbdev_set_suspend(struct drm_device *dev, int state)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	if (!dev_priv->fbdev)
 		return;
 
@@ -289,13 +289,13 @@ MODULE_LICENSE("GPL and additional rights");
 
 void intel_fb_output_poll_changed(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
 
 void intel_fb_restore_mode(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (INTEL_INFO(dev)->num_pipes == 0)
 		return;
-- 
1.8.1.5

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

[PATCH 7/7] drm: Remove some unused stuff from drm_plane

[ville.syrjala]

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

There's a bunch of unused members inside drm_plane, bloating the size of
the structure needlessly. Eliminate them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c |  2 +-
 include/drm/drm_crtc.h     | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d7c449f..e591914 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 
 	plane_resp->plane_id = plane->base.id;
 	plane_resp->possible_crtcs = plane->possible_crtcs;
-	plane_resp->gamma_size = plane->gamma_size;
+	plane_resp->gamma_size = 0;
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index adb3f9b..99420c4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -654,9 +654,6 @@ struct drm_plane_funcs {
  * @format_count: number of formats supported
  * @crtc: currently bound CRTC
  * @fb: currently bound fb
- * @gamma_size: size of gamma table
- * @gamma_store: gamma correction table
- * @enabled: enabled flag
  * @funcs: helper functions
  * @helper_private: storage for drver layer
  * @properties: property tracking for this plane
@@ -674,14 +671,7 @@ struct drm_plane {
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
-	/* CRTC gamma size for reporting to userspace */
-	uint32_t gamma_size;
-	uint16_t *gamma_store;
-
-	bool enabled;
-
 	const struct drm_plane_funcs *funcs;
-	void *helper_private;
 
 	struct drm_object_properties properties;
 };
-- 
1.8.1.5

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

Re: [PATCH 7/7] drm: Remove some unused stuff from drm_plane

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Wednesday 08 May 2013 12:55:22 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's a bunch of unused members inside drm_plane, bloating the size of
> the structure needlessly. Eliminate them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c |  2 +-
>  include/drm/drm_crtc.h     | 10 ----------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d7c449f..e591914 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void
> *data,
> 
>  	plane_resp->plane_id = plane->base.id;
>  	plane_resp->possible_crtcs = plane->possible_crtcs;
> -	plane_resp->gamma_size = plane->gamma_size;
> +	plane_resp->gamma_size = 0;
> 
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index adb3f9b..99420c4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -654,9 +654,6 @@ struct drm_plane_funcs {
>   * @format_count: number of formats supported
>   * @crtc: currently bound CRTC
>   * @fb: currently bound fb
> - * @gamma_size: size of gamma table
> - * @gamma_store: gamma correction table
> - * @enabled: enabled flag
>   * @funcs: helper functions
>   * @helper_private: storage for drver layer

Shouldn't the above line be removed as well ?

>   * @properties: property tracking for this plane
> @@ -674,14 +671,7 @@ struct drm_plane {
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> 
> -	/* CRTC gamma size for reporting to userspace */
> -	uint32_t gamma_size;
> -	uint16_t *gamma_store;
> -
> -	bool enabled;
> -
>  	const struct drm_plane_funcs *funcs;
> -	void *helper_private;
> 
>  	struct drm_object_properties properties;
>  };

-- 
Regards,

Laurent Pinchart

[PATCH v2] drm: Remove some unused stuff from drm_plane

[ville.syrjala]

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

There's a bunch of unused members inside drm_plane, bloating the size of
the structure needlessly. Eliminate them.

v2: Remove all of it from kernel-doc too

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c |  2 +-
 include/drm/drm_crtc.h     | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d7c449f..e591914 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 
 	plane_resp->plane_id = plane->base.id;
 	plane_resp->possible_crtcs = plane->possible_crtcs;
-	plane_resp->gamma_size = plane->gamma_size;
+	plane_resp->gamma_size = 0;
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index adb3f9b..e11c6bc 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -654,11 +654,7 @@ struct drm_plane_funcs {
  * @format_count: number of formats supported
  * @crtc: currently bound CRTC
  * @fb: currently bound fb
- * @gamma_size: size of gamma table
- * @gamma_store: gamma correction table
- * @enabled: enabled flag
  * @funcs: helper functions
- * @helper_private: storage for drver layer
  * @properties: property tracking for this plane
  */
 struct drm_plane {
@@ -674,14 +670,7 @@ struct drm_plane {
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
-	/* CRTC gamma size for reporting to userspace */
-	uint32_t gamma_size;
-	uint16_t *gamma_store;
-
-	bool enabled;
-
 	const struct drm_plane_funcs *funcs;
-	void *helper_private;
 
 	struct drm_object_properties properties;
 };
-- 
1.8.1.5

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

Re: [PATCH v2] drm: Remove some unused stuff from drm_plane

[Laurent Pinchart]

On Wednesday 08 May 2013 16:40:54 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's a bunch of unused members inside drm_plane, bloating the size of
> the structure needlessly. Eliminate them.
> 
> v2: Remove all of it from kernel-doc too
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_crtc.c |  2 +-
>  include/drm/drm_crtc.h     | 11 -----------
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d7c449f..e591914 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void
> *data,
> 
>  	plane_resp->plane_id = plane->base.id;
>  	plane_resp->possible_crtcs = plane->possible_crtcs;
> -	plane_resp->gamma_size = plane->gamma_size;
> +	plane_resp->gamma_size = 0;
> 
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index adb3f9b..e11c6bc 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -654,11 +654,7 @@ struct drm_plane_funcs {
>   * @format_count: number of formats supported
>   * @crtc: currently bound CRTC
>   * @fb: currently bound fb
> - * @gamma_size: size of gamma table
> - * @gamma_store: gamma correction table
> - * @enabled: enabled flag
>   * @funcs: helper functions
> - * @helper_private: storage for drver layer
>   * @properties: property tracking for this plane
>   */
>  struct drm_plane {
> @@ -674,14 +670,7 @@ struct drm_plane {
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> 
> -	/* CRTC gamma size for reporting to userspace */
> -	uint32_t gamma_size;
> -	uint16_t *gamma_store;
> -
> -	bool enabled;
> -
>  	const struct drm_plane_funcs *funcs;
> -	void *helper_private;
> 
>  	struct drm_object_properties properties;
>  };
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] drm: Remove some unused stuff from drm_plane

[Laurent Pinchart]

Hello,

On Wednesday 08 May 2013 15:52:10 Laurent Pinchart wrote:
> On Wednesday 08 May 2013 16:40:54 ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > There's a bunch of unused members inside drm_plane, bloating the size of
> > the structure needlessly. Eliminate them.
> > 
> > v2: Remove all of it from kernel-doc too
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Wow, I've managed to review the patch and miss that it broke compilation of 
the shmob-drm driver :-( I'll send a patch to fix it.

> > ---
> > 
> >  drivers/gpu/drm/drm_crtc.c |  2 +-
> >  include/drm/drm_crtc.h     | 11 -----------
> >  2 files changed, 1 insertion(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index d7c449f..e591914 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void
> > *data,
> > 
> >  	plane_resp->plane_id = plane->base.id;
> >  	plane_resp->possible_crtcs = plane->possible_crtcs;
> > 
> > -	plane_resp->gamma_size = plane->gamma_size;
> > +	plane_resp->gamma_size = 0;
> > 
> >  	/*
> >  	
> >  	 * This ioctl is called twice, once to determine how much space is
> > 
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index adb3f9b..e11c6bc 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -654,11 +654,7 @@ struct drm_plane_funcs {
> >   * @format_count: number of formats supported
> >   * @crtc: currently bound CRTC
> >   * @fb: currently bound fb
> > - * @gamma_size: size of gamma table
> > - * @gamma_store: gamma correction table
> > - * @enabled: enabled flag
> >   * @funcs: helper functions
> > - * @helper_private: storage for drver layer
> >   * @properties: property tracking for this plane
> >   */
> >  
> >  struct drm_plane {
> > @@ -674,14 +670,7 @@ struct drm_plane {
> >  	struct drm_crtc *crtc;
> >  	struct drm_framebuffer *fb;
> > 
> > -	/* CRTC gamma size for reporting to userspace */
> > -	uint32_t gamma_size;
> > -	uint16_t *gamma_store;
> > -
> > -	bool enabled;
> > -
> >  	const struct drm_plane_funcs *funcs;
> > -	void *helper_private;
> > 
> >  	struct drm_object_properties properties;
> >  };
-- 
Regards,

Laurent Pinchart

Re: [Intel-gfx] [PATCH 7/7] drm: Remove some unused stuff from drm_plane

[Imre Deak]

On Wed, 2013-05-08 at 12:55 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's a bunch of unused members inside drm_plane, bloating the size of
> the structure needlessly. Eliminate them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

On the series:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/drm_crtc.c |  2 +-
>  include/drm/drm_crtc.h     | 10 ----------
>  2 files changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d7c449f..e591914 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1739,7 +1739,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  
>  	plane_resp->plane_id = plane->base.id;
>  	plane_resp->possible_crtcs = plane->possible_crtcs;
> -	plane_resp->gamma_size = plane->gamma_size;
> +	plane_resp->gamma_size = 0;
>  
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index adb3f9b..99420c4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -654,9 +654,6 @@ struct drm_plane_funcs {
>   * @format_count: number of formats supported
>   * @crtc: currently bound CRTC
>   * @fb: currently bound fb
> - * @gamma_size: size of gamma table
> - * @gamma_store: gamma correction table
> - * @enabled: enabled flag
>   * @funcs: helper functions
>   * @helper_private: storage for drver layer
>   * @properties: property tracking for this plane
> @@ -674,14 +671,7 @@ struct drm_plane {
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
>  
> -	/* CRTC gamma size for reporting to userspace */
> -	uint32_t gamma_size;
> -	uint16_t *gamma_store;
> -
> -	bool enabled;
> -
>  	const struct drm_plane_funcs *funcs;
> -	void *helper_private;
>  
>  	struct drm_object_properties properties;
>  };


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel