All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Two new drm crtc properties
@ 2014-03-10  5:10 akash.goel
  2014-03-10  5:10 ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: akash.goel @ 2014-03-10  5:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

Added 2 new drm crtc properties. One property
provides control to vary the PIPESRC value. With this the
size of Pipe's output or Panel fitter input can be varied.
The other property provides control to vary the size of
horizontal & vertical borders. With this the size of the
Panel fitter output or display window could be controlled. 

Akash Goel (3):
  drm/i915: Initialized 'set_property' fn pointer field of
    intel_crtc_funcs structure
  drm/i915: New drm crtc property for varying the Pipe Src size
  drm/i915: New drm crtc property for varying the size of borders

 drivers/gpu/drm/i915/i915_drv.h      |  13 ++++
 drivers/gpu/drm/i915/intel_display.c |  72 +++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 147 +++++++++++++++++++++++++++++++++++
 4 files changed, 236 insertions(+)

-- 
1.8.5.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure
  2014-03-10  5:10 [PATCH 0/3] Two new drm crtc properties akash.goel
@ 2014-03-10  5:10 ` akash.goel
  2014-03-10  5:10 ` [PATCH 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: akash.goel @ 2014-03-10  5:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch defines a new function & assigns that to the 'set_property'
function pointer field of the 'intel_crtc_funcs' structure.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8494194..5dfe156 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10431,6 +10431,14 @@ out_config:
 	return ret;
 }
 
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+		struct drm_property *property, uint64_t val)
+{
+	int ret = -ENOENT;
+
+	return ret;
+}
+
 static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.cursor_set = intel_crtc_cursor_set,
 	.cursor_move = intel_crtc_cursor_move,
@@ -10438,6 +10446,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property,
 };
 
 static void intel_cpu_pll_init(struct drm_device *dev)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/3] drm/i915: New drm crtc property for varying the Pipe Src size
  2014-03-10  5:10 [PATCH 0/3] Two new drm crtc properties akash.goel
  2014-03-10  5:10 ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
@ 2014-03-10  5:10 ` akash.goel
  2014-03-10  5:10 ` [PATCH 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
  2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  3 siblings, 0 replies; 26+ messages in thread
From: akash.goel @ 2014-03-10  5:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the Pipe Src size
or the Panel fitter input size. Pipe Src controls the size that is
scaled from.
This will allow to dynamically flip (without modeset) the frame buffers
of different resolutions

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0d90ef..6f3af15 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1608,6 +1608,12 @@ typedef struct drm_i915_private {
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * PIPESRC or Panel fitter input size
+	 */
+	struct drm_property *input_size_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5dfe156..30374e9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10434,8 +10434,38 @@ out_config:
 static int intel_crtc_set_property(struct drm_crtc *crtc,
 		struct drm_property *property, uint64_t val)
 {
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int ret = -ENOENT;
 
+	if (property == dev_priv->input_size_property) {
+		int new_width = (int)((val >> 16) & 0xffff);
+		int new_height = (int)(val & 0xffff);
+
+		if ((new_width == intel_crtc->config.pipe_src_w) &&
+		    (new_height == intel_crtc->config.pipe_src_h))
+			return 0;
+
+		intel_crtc->config.pipe_src_w = new_width;
+		intel_crtc->config.pipe_src_h = new_height;
+
+		intel_crtc->config.requested_mode.hdisplay = new_width;
+		intel_crtc->config.requested_mode.vdisplay = new_height;
+
+		crtc->mode.hdisplay = new_width;
+		crtc->mode.vdisplay = new_height;
+
+		/* pipesrc controls the size that is scaled from, which should
+		* always be the user's requested size.
+		*/
+		I915_WRITE(PIPESRC(intel_crtc->pipe),
+			((intel_crtc->config.pipe_src_w - 1) << 16) |
+			 (intel_crtc->config.pipe_src_h - 1));
+
+		return 0;
+	}
+
 	return ret;
 }
 
@@ -10586,6 +10616,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
+
+	if (!dev_priv->input_size_property)
+		dev_priv->input_size_property =
+			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
+
+	if (dev_priv->input_size_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->input_size_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-10  5:10 [PATCH 0/3] Two new drm crtc properties akash.goel
  2014-03-10  5:10 ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
  2014-03-10  5:10 ` [PATCH 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
@ 2014-03-10  5:10 ` akash.goel
  2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  3 siblings, 0 replies; 26+ messages in thread
From: akash.goel @ 2014-03-10  5:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of
the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c |  28 ++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 147 +++++++++++++++++++++++++++++++++++
 4 files changed, 184 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *output_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 30374e9..2d213db 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10437,7 +10437,16 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
+
+	if (property == dev_priv->output_border_property) {
+		if (val == (uint64_t)intel_crtc->border_size)
+			return 0;
+
+		intel_crtc->border_size = (uint32_t)val;
+		intel_crtc->border_size_changed = 1;
+
+		goto done;
+	}
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff);
@@ -10466,7 +10475,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
+
+done:
+	if (crtc)
+		intel_crtc_restore_mode(crtc);
+
+	return 0;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
@@ -10625,6 +10640,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (!dev_priv->output_border_property)
+		dev_priv->output_border_property =
+			drm_property_create_range(dev, 0, "border size", 0, 0xFFFFFFFF);
+
+	if (dev_priv->output_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->output_border_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..68eec38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,10 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* border info for the output/display window */
+	bool border_size_changed;
+	uint32_t border_size;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb05840..5b6b26a 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -42,6 +42,57 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 tot_width, tot_height;
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	tot_width  = adjusted_mode->crtc_hdisplay;
+	tot_height = adjusted_mode->crtc_vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	x = y = 0;
+
+	if (tot_width < dst_width) {
+		DRM_ERROR("width is too big\n");
+		return;
+	} else if (dst_width & 1) {
+		DRM_ERROR("width must be even\n");
+		return;
+	} else if (tot_height < dst_height) {
+		DRM_ERROR("height is too big\n");
+		return;
+	} else if (dst_height & 1) {
+		DRM_ERROR("height must be even\n");
+		return;
+	}
+
+	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
+	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+
+	intel_crtc->border_size_changed = 0;
+}
+
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
@@ -55,6 +106,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if size of borders has changed and border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed &&
+			intel_crtc->border_size)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
@@ -247,6 +305,88 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->crtc_hdisplay;
+	tot_height = adjusted_mode->crtc_vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
+#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+			DRM_ERROR("width is too small\n");
+			return;
+	} else if (tot_width < dst_width) {
+			DRM_ERROR("width is too big\n");
+			return;
+	} else if (dst_width & 1) {
+			DRM_ERROR("width must be even\n");
+			return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+			DRM_ERROR("height is too small\n");
+			return;
+	} else if (tot_height < dst_height) {
+			DRM_ERROR("height is too big\n");
+			return;
+	} else if (dst_height & 1) {
+			DRM_ERROR("height must be even\n");
+			return;
+	}
+
+	if (dst_width != tot_width)
+		centre_horizontally(adjusted_mode, dst_width);
+	if (dst_height != tot_height)
+		centre_vertically(adjusted_mode, dst_height);
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
+
+	intel_crtc->border_size_changed = 0;
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +397,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if size of borders has changed and border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed &&
+			intel_crtc->border_size)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/3] Two new drm crtc properties
  2014-03-10  5:10 [PATCH 0/3] Two new drm crtc properties akash.goel
                   ` (2 preceding siblings ...)
  2014-03-10  5:10 ` [PATCH 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
@ 2014-03-10  5:14 ` Daniel Vetter
  2014-03-10  5:17   ` Goel, Akash
                     ` (2 more replies)
  3 siblings, 3 replies; 26+ messages in thread
From: Daniel Vetter @ 2014-03-10  5:14 UTC (permalink / raw)
  To: akash.goel; +Cc: intel-gfx

On Mon, Mar 10, 2014 at 10:40:50AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> Added 2 new drm crtc properties. One property
> provides control to vary the PIPESRC value. With this the
> size of Pipe's output or Panel fitter input can be varied.
> The other property provides control to vary the size of
> horizontal & vertical borders. With this the size of the
> Panel fitter output or display window could be controlled. 
> 
> Akash Goel (3):
>   drm/i915: Initialized 'set_property' fn pointer field of
>     intel_crtc_funcs structure
>   drm/i915: New drm crtc property for varying the Pipe Src size
>   drm/i915: New drm crtc property for varying the size of borders

Hm, where are the igt testcases for this feature? I expect some fun
interactions at least with pageflips/modesets vs. setting this. The
vblank timing code should be fine, since it always deals with adjusted
timings.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/3] Two new drm crtc properties
  2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
@ 2014-03-10  5:17   ` Goel, Akash
  2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
  2014-03-21 19:53   ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  2 siblings, 0 replies; 26+ messages in thread
From: Goel, Akash @ 2014-03-10  5:17 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Sorry, will send across the corresponding IGT test case also shortly.

Best regards
Akash

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, March 10, 2014 10:44 AM
To: Goel, Akash
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/3] Two new drm crtc properties

On Mon, Mar 10, 2014 at 10:40:50AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> Added 2 new drm crtc properties. One property provides control to vary 
> the PIPESRC value. With this the size of Pipe's output or Panel fitter 
> input can be varied.
> The other property provides control to vary the size of horizontal & 
> vertical borders. With this the size of the Panel fitter output or 
> display window could be controlled.
> 
> Akash Goel (3):
>   drm/i915: Initialized 'set_property' fn pointer field of
>     intel_crtc_funcs structure
>   drm/i915: New drm crtc property for varying the Pipe Src size
>   drm/i915: New drm crtc property for varying the size of borders

Hm, where are the igt testcases for this feature? I expect some fun interactions at least with pageflips/modesets vs. setting this. The vblank timing code should be fine, since it always deals with adjusted timings.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure
  2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  2014-03-10  5:17   ` Goel, Akash
@ 2014-03-11 12:54   ` akash.goel
  2014-03-11 12:54     ` [PATCH v2 0/3] Two new drm crtc properties akash.goel
                       ` (2 more replies)
  2014-03-21 19:53   ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  2 siblings, 3 replies; 26+ messages in thread
From: akash.goel @ 2014-03-11 12:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch defines a new function & assigns that to the 'set_property'
function pointer field of the 'intel_crtc_funcs' structure.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8494194..5dfe156 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10431,6 +10431,14 @@ out_config:
 	return ret;
 }
 
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+		struct drm_property *property, uint64_t val)
+{
+	int ret = -ENOENT;
+
+	return ret;
+}
+
 static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.cursor_set = intel_crtc_cursor_set,
 	.cursor_move = intel_crtc_cursor_move,
@@ -10438,6 +10446,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property,
 };
 
 static void intel_cpu_pll_init(struct drm_device *dev)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 0/3] Two new drm crtc properties
  2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
@ 2014-03-11 12:54     ` akash.goel
  2014-03-21 19:15       ` Goel, Akash
  2014-03-11 12:54     ` [PATCH v2 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
  2014-03-11 12:54     ` [PATCH v2 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
  2 siblings, 1 reply; 26+ messages in thread
From: akash.goel @ 2014-03-11 12:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

Added 2 new drm crtc properties. One property
provides control to vary the PIPESRC value. With this the
size of Pipe's output or Panel fitter input can be varied.
The other property provides control to vary the size of
horizontal & vertical borders. With this the size of the
Panel fitter output or display window could be controlled. 
v2: Added new checks and disabled the fb pitch mismatch check
for VLV & HSW

Akash Goel (3):
  drm/i915: Initialized 'set_property' fn pointer field of
    intel_crtc_funcs structure
  drm/i915: New drm crtc property for varying the Pipe Src size
  drm/i915: New drm crtc property for varying the size of borders

 drivers/gpu/drm/i915/i915_drv.h      |  13 +++
 drivers/gpu/drm/i915/intel_display.c |  93 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 159 +++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+), 2 deletions(-)

-- 
1.8.5.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 2/3] drm/i915: New drm crtc property for varying the Pipe Src size
  2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
  2014-03-11 12:54     ` [PATCH v2 0/3] Two new drm crtc properties akash.goel
@ 2014-03-11 12:54     ` akash.goel
  2014-03-11 12:54     ` [PATCH v2 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
  2 siblings, 0 replies; 26+ messages in thread
From: akash.goel @ 2014-03-11 12:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the Pipe Src size
or the Panel fitter input size. Pipe Src controls the size that is
scaled from.
This will allow to dynamically flip (without modeset) the frame buffers
of different resolutions

v2: Added a check to fail the set property call if Panel fitter is
disabled & new PIPESRC programming do not match with PIPE timings.
Removed the pitch mismatch check on frame buffer, when being flipped.
This is currently done only for VLV/HSW.

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++
 drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0d90ef..6f3af15 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1608,6 +1608,12 @@ typedef struct drm_i915_private {
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * PIPESRC or Panel fitter input size
+	 */
+	struct drm_property *input_size_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5dfe156..8772390 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8935,8 +8935,18 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	 * Note that pitch changes could also affect these register.
 	 */
 	if (INTEL_INFO(dev)->gen > 3 &&
-	    (fb->offsets[0] != crtc->fb->offsets[0] ||
-	     fb->pitches[0] != crtc->fb->pitches[0]))
+	    (fb->offsets[0] != crtc->fb->offsets[0]))
+		return -EINVAL;
+
+	/*
+	 * Bypassing the fb pitch check for VLV/HSW, as purportedly there
+	 * is a dynamic flip support in VLV/HSW. This will allow to
+	 * flip fbs of different resolutions without doing a modeset.
+	 * TBD, confirm the same for other newer gen platforms also.
+	 */
+	if (INTEL_INFO(dev)->gen > 3 &&
+	    !IS_VALLEYVIEW(dev) && !IS_HASWELL(dev) &&
+	    (fb->pitches[0] != crtc->fb->pitches[0]))
 		return -EINVAL;
 
 	if (i915_terminally_wedged(&dev_priv->gpu_error))
@@ -10434,8 +10444,45 @@ out_config:
 static int intel_crtc_set_property(struct drm_crtc *crtc,
 		struct drm_property *property, uint64_t val)
 {
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int ret = -ENOENT;
 
+	if (property == dev_priv->input_size_property) {
+		int new_width = (int)((val >> 16) & 0xffff);
+		int new_height = (int)(val & 0xffff);
+
+		if ((new_width == intel_crtc->config.pipe_src_w) &&
+		    (new_height == intel_crtc->config.pipe_src_h))
+			return 0;
+
+		if ((!intel_crtc->config.gmch_pfit.control) &&
+		    ((intel_crtc->config.adjusted_mode.hdisplay) != (new_width) ||
+		     (intel_crtc->config.adjusted_mode.vdisplay) != (new_height))) {
+			DRM_ERROR("PIPESRC mismatch with Pipe timings & PF is disabled\n");
+			return -EINVAL;
+		}
+
+		intel_crtc->config.pipe_src_w = new_width;
+		intel_crtc->config.pipe_src_h = new_height;
+
+		intel_crtc->config.requested_mode.hdisplay = new_width;
+		intel_crtc->config.requested_mode.vdisplay = new_height;
+
+		crtc->mode.hdisplay = new_width;
+		crtc->mode.vdisplay = new_height;
+
+		/* pipesrc controls the size that is scaled from, which should
+		* always be the user's requested size.
+		*/
+		I915_WRITE(PIPESRC(intel_crtc->pipe),
+			((intel_crtc->config.pipe_src_w - 1) << 16) |
+			 (intel_crtc->config.pipe_src_h - 1));
+
+		return 0;
+	}
+
 	return ret;
 }
 
@@ -10586,6 +10633,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
+
+	if (!dev_priv->input_size_property)
+		dev_priv->input_size_property =
+			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
+
+	if (dev_priv->input_size_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->input_size_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
  2014-03-11 12:54     ` [PATCH v2 0/3] Two new drm crtc properties akash.goel
  2014-03-11 12:54     ` [PATCH v2 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
@ 2014-03-11 12:54     ` akash.goel
  2014-03-23  8:56       ` [PATCH v3] " akash.goel
  2 siblings, 1 reply; 26+ messages in thread
From: akash.goel @ 2014-03-11 12:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of
the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

v2: Added a new check for the invalid border size input

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c |  28 +++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 159 +++++++++++++++++++++++++++++++++++
 4 files changed, 196 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *output_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8772390..68ec5b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10447,7 +10447,16 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
+
+	if (property == dev_priv->output_border_property) {
+		if (val == (uint64_t)intel_crtc->border_size)
+			return 0;
+
+		intel_crtc->border_size = (uint32_t)val;
+		intel_crtc->border_size_changed = 1;
+
+		goto done;
+	}
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff);
@@ -10483,7 +10492,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
+
+done:
+	if (crtc)
+		intel_crtc_restore_mode(crtc);
+
+	return 0;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
@@ -10642,6 +10657,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (!dev_priv->output_border_property)
+		dev_priv->output_border_property =
+			drm_property_create_range(dev, 0, "border size", 0, 0xFFFFFFFF);
+
+	if (dev_priv->output_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->output_border_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..68eec38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,10 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* border info for the output/display window */
+	bool border_size_changed;
+	uint32_t border_size;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb05840..1d8444d 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -42,6 +42,63 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 tot_width, tot_height;
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	tot_width  = adjusted_mode->crtc_hdisplay;
+	tot_height = adjusted_mode->crtc_vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		goto out;
+	}
+
+	x = y = 0;
+
+	if (tot_width < dst_width) {
+		DRM_ERROR("width is too big\n");
+		goto out;
+	} else if (dst_width & 1) {
+		DRM_ERROR("width must be even\n");
+		goto out;
+	} else if (tot_height < dst_height) {
+		DRM_ERROR("height is too big\n");
+		goto out;
+	} else if (dst_height & 1) {
+		DRM_ERROR("height must be even\n");
+		goto out;
+	}
+
+	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
+	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+
+out:
+	intel_crtc->border_size_changed = 0;
+}
+
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
@@ -55,6 +112,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if size of borders has changed and border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed &&
+			intel_crtc->border_size)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
@@ -247,6 +311,94 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->crtc_hdisplay;
+	tot_height = adjusted_mode->crtc_vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		goto out;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
+#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+			DRM_ERROR("width is too small\n");
+			goto out;
+	} else if (tot_width < dst_width) {
+			DRM_ERROR("width is too big\n");
+			goto out;
+	} else if (dst_width & 1) {
+			DRM_ERROR("width must be even\n");
+			goto out;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+			DRM_ERROR("height is too small\n");
+			goto out;
+	} else if (tot_height < dst_height) {
+			DRM_ERROR("height is too big\n");
+			goto out;
+	} else if (dst_height & 1) {
+			DRM_ERROR("height must be even\n");
+			goto out;
+	}
+
+	if (dst_width != tot_width)
+		centre_horizontally(adjusted_mode, dst_width);
+	if (dst_height != tot_height)
+		centre_vertically(adjusted_mode, dst_height);
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
+
+out:
+	intel_crtc->border_size_changed = 0;
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +409,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if size of borders has changed and border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed &&
+			intel_crtc->border_size)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/3] Two new drm crtc properties
  2014-03-11 12:54     ` [PATCH v2 0/3] Two new drm crtc properties akash.goel
@ 2014-03-21 19:15       ` Goel, Akash
  0 siblings, 0 replies; 26+ messages in thread
From: Goel, Akash @ 2014-03-21 19:15 UTC (permalink / raw)
  To: Ville Syrjälä (ville.syrjala@linux.intel.com); +Cc: intel-gfx

Hi Ville,

Please could you review this patch & provide a feedback.
This is as per your suggestions & our earlier discussions. 

Regards,
Sourab

-----Original Message-----
From: Goel, Akash 
Sent: Tuesday, March 11, 2014 6:24 PM
To: intel-gfx@lists.freedesktop.org
Cc: Purushothaman, Vijay A; G, Pallavi; Goel, Akash
Subject: [PATCH v2 0/3] Two new drm crtc properties

From: Akash Goel <akash.goel@intel.com>

Added 2 new drm crtc properties. One property provides control to vary the PIPESRC value. With this the size of Pipe's output or Panel fitter input can be varied.
The other property provides control to vary the size of horizontal & vertical borders. With this the size of the Panel fitter output or display window could be controlled. 
v2: Added new checks and disabled the fb pitch mismatch check for VLV & HSW

Akash Goel (3):
  drm/i915: Initialized 'set_property' fn pointer field of
    intel_crtc_funcs structure
  drm/i915: New drm crtc property for varying the Pipe Src size
  drm/i915: New drm crtc property for varying the size of borders

 drivers/gpu/drm/i915/i915_drv.h      |  13 +++
 drivers/gpu/drm/i915/intel_display.c |  93 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 159 +++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+), 2 deletions(-)

--
1.8.5.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/3] Two new drm crtc properties
  2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
  2014-03-10  5:17   ` Goel, Akash
  2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
@ 2014-03-21 19:53   ` Daniel Vetter
  2014-03-22  4:29     ` Goel, Akash
  2 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2014-03-21 19:53 UTC (permalink / raw)
  To: akash.goel; +Cc: intel-gfx

On Mon, Mar 10, 2014 at 6:14 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 10, 2014 at 10:40:50AM +0530, akash.goel@intel.com wrote:
>> From: Akash Goel <akash.goel@intel.com>
>>
>> Added 2 new drm crtc properties. One property
>> provides control to vary the PIPESRC value. With this the
>> size of Pipe's output or Panel fitter input can be varied.
>> The other property provides control to vary the size of
>> horizontal & vertical borders. With this the size of the
>> Panel fitter output or display window could be controlled.
>>
>> Akash Goel (3):
>>   drm/i915: Initialized 'set_property' fn pointer field of
>>     intel_crtc_funcs structure
>>   drm/i915: New drm crtc property for varying the Pipe Src size
>>   drm/i915: New drm crtc property for varying the size of borders
>
> Hm, where are the igt testcases for this feature? I expect some fun
> interactions at least with pageflips/modesets vs. setting this. The
> vblank timing code should be fine, since it always deals with adjusted
> timings.

Follow-up from my side ... could we just expose this through primary
planes, or is the damn panel fitter at the wrong spot in the pipeline?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/3] Two new drm crtc properties
  2014-03-21 19:53   ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
@ 2014-03-22  4:29     ` Goel, Akash
  0 siblings, 0 replies; 26+ messages in thread
From: Goel, Akash @ 2014-03-22  4:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

Actually since the Panel fitter is at the Pipe level and also the PIPESRC register, this has been exposed through crtc property. But with crtc also, it is kind of associated with primary planes only.

Best Regards
Akash

-----Original Message-----
From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Saturday, March 22, 2014 1:24 AM
To: Goel, Akash
Cc: intel-gfx
Subject: Re: [Intel-gfx] [PATCH 0/3] Two new drm crtc properties

On Mon, Mar 10, 2014 at 6:14 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 10, 2014 at 10:40:50AM +0530, akash.goel@intel.com wrote:
>> From: Akash Goel <akash.goel@intel.com>
>>
>> Added 2 new drm crtc properties. One property provides control to 
>> vary the PIPESRC value. With this the size of Pipe's output or Panel 
>> fitter input can be varied.
>> The other property provides control to vary the size of horizontal & 
>> vertical borders. With this the size of the Panel fitter output or 
>> display window could be controlled.
>>
>> Akash Goel (3):
>>   drm/i915: Initialized 'set_property' fn pointer field of
>>     intel_crtc_funcs structure
>>   drm/i915: New drm crtc property for varying the Pipe Src size
>>   drm/i915: New drm crtc property for varying the size of borders
>
> Hm, where are the igt testcases for this feature? I expect some fun 
> interactions at least with pageflips/modesets vs. setting this. The 
> vblank timing code should be fine, since it always deals with adjusted 
> timings.

Follow-up from my side ... could we just expose this through primary planes, or is the damn panel fitter at the wrong spot in the pipeline?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-11 12:54     ` [PATCH v2 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
@ 2014-03-23  8:56       ` akash.goel
  2014-03-26  3:55         ` [PATCH v4 3/3] " akash.goel
  0 siblings, 1 reply; 26+ messages in thread
From: akash.goel @ 2014-03-23  8:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of
the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

v2: Added a new check for the invalid border size input

v3: Fixed bugs in output window calculation
Removed superfluous checks

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c |  28 +++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_panel.c   | 180 ++++++++++++++++++++++++++++++++---
 4 files changed, 204 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *output_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8772390..68ec5b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10447,7 +10447,16 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
+
+	if (property == dev_priv->output_border_property) {
+		if (val == (uint64_t)intel_crtc->border_size)
+			return 0;
+
+		intel_crtc->border_size = (uint32_t)val;
+		intel_crtc->border_size_changed = 1;
+
+		goto done;
+	}
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff);
@@ -10483,7 +10492,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
+
+done:
+	if (crtc)
+		intel_crtc_restore_mode(crtc);
+
+	return 0;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
@@ -10642,6 +10657,15 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (!dev_priv->output_border_property)
+		dev_priv->output_border_property =
+			drm_property_create_range(dev, 0, "border size", 0, 0xFFFFFFFF);
+
+	if (dev_priv->output_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->output_border_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..68eec38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,10 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* border info for the output/display window */
+	bool border_size_changed;
+	uint32_t border_size;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb05840..ca61a17 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -29,10 +29,25 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
+#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
 
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
+static inline u32 panel_fitter_scaling(u32 source, u32 target)
+{
+	/*
+	 * Floating point operation is not supported. So the FACTOR
+	 * is defined, which can avoid the floating point computation
+	 * when calculating the panel ratio.
+	 */
+#define ACCURACY 12
+#define FACTOR (1 << ACCURACY)
+	u32 ratio = source * FACTOR / target;
+	return (FACTOR * ratio + FACTOR/2) / FACTOR;
+}
+
 void
 intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -42,6 +57,63 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width  = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		goto out;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		goto out;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		goto out;
+	}
+
+	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
+	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+
+out:
+	intel_crtc->border_size_changed = 0;
+}
+
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
@@ -55,6 +127,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if size of borders has changed and border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed ||
+			intel_crtc->border_size)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
@@ -157,19 +236,6 @@ centre_vertically(struct drm_display_mode *mode,
 	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
 }
 
-static inline u32 panel_fitter_scaling(u32 source, u32 target)
-{
-	/*
-	 * Floating point operation is not supported. So the FACTOR
-	 * is defined, which can avoid the floating point computation
-	 * when calculating the panel ratio.
-	 */
-#define ACCURACY 12
-#define FACTOR (1 << ACCURACY)
-	u32 ratio = source * FACTOR / target;
-	return (FACTOR * ratio + FACTOR/2) / FACTOR;
-}
-
 static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
 			      u32 *pfit_control)
 {
@@ -247,6 +313,87 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		goto out;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		goto out;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		goto out;
+	}
+
+	if (dst_width != tot_width)
+		centre_horizontally(adjusted_mode, dst_width);
+	if (dst_height != tot_height)
+		centre_vertically(adjusted_mode, dst_height);
+
+	/* Don't enable the Panel fitter, if no scaling is needed */
+	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
+		DRM_DEBUG_KMS("Skipping enabling of Panel fitter\n");
+		goto out;
+	}
+
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
+
+out:
+	intel_crtc->border_size_changed = 0;
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +404,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if size of borders has changed or border size is
+	   not zero, otherwise fall through the regular path */
+	if (intel_crtc->border_size_changed ||
+			intel_crtc->border_size)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-23  8:56       ` [PATCH v3] " akash.goel
@ 2014-03-26  3:55         ` akash.goel
  2014-04-07  4:06           ` Goel, Akash
  2014-04-08 16:28           ` Ville Syrjälä
  0 siblings, 2 replies; 26+ messages in thread
From: akash.goel @ 2014-03-26  3:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of
the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

v2: Added a new check for the invalid border size input

v3: Fixed bugs in output window calculation
Removed superfluous checks

v4: Added the capability to forecfully enable the Panel fitter.
The property value is of 64 bits, first 32 bits are used for
border dimensions. The 33rd bit can be used to forcefully
enable the panel fitter. This is useful for Panels which
do not override the User specified Pipe timings.

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   5 +
 drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
 4 files changed, 211 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *output_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7149123..a217f25 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10447,7 +10447,23 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
+
+	if (property == dev_priv->output_border_property) {
+		if ((val == (uint64_t)intel_crtc->border_size) &&
+		    (((val >> 32) & 0x1) == intel_crtc->pfit_enabled))
+			return 0;
+
+		intel_crtc->border_size = (uint32_t)val;
+		intel_crtc->pfit_enabled = (val >> 32) & 0x1;
+		if ((intel_crtc->border_size != 0) &&
+		    (!intel_crtc->pfit_enabled)) {
+			DRM_ERROR("Wrong setting, expect Pfit to be enabled when "
+				  "applying borders\n");
+			return -EINVAL;
+		}
+
+		goto done;
+	}
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff);
@@ -10488,7 +10504,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
+
+done:
+	if (crtc)
+		intel_crtc_restore_mode(crtc);
+
+	return 0;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
@@ -10641,12 +10663,23 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	if (!dev_priv->input_size_property)
 		dev_priv->input_size_property =
-			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
+			drm_property_create_range(dev, 0, "input size",
+						0, 0xFFFFFFFF);
 
 	if (dev_priv->input_size_property)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (!dev_priv->output_border_property)
+		dev_priv->output_border_property =
+			drm_property_create_range(dev, 0, "border size",
+						0, (uint64_t)0x1FFFFFFFF);
+
+	if (dev_priv->output_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->output_border_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..3cfc9da 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,11 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* for forceful enabling of panel fitter */
+	bool pfit_enabled;
+	/* border info for the output/display window */
+	uint32_t border_size;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb05840..86a486e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -29,10 +29,25 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
+#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
 
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
+static inline u32 panel_fitter_scaling(u32 source, u32 target)
+{
+	/*
+	 * Floating point operation is not supported. So the FACTOR
+	 * is defined, which can avoid the floating point computation
+	 * when calculating the panel ratio.
+	 */
+#define ACCURACY 12
+#define FACTOR (1 << ACCURACY)
+	u32 ratio = source * FACTOR / target;
+	return (FACTOR * ratio + FACTOR/2) / FACTOR;
+}
+
 void
 intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width  = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
+	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+}
+
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
@@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if User wants to apply the borders, or wants to forcefully
+	   enable the panel fitter, otherwise fall through the regular path */
+	if (intel_crtc->pfit_enabled ||
+			intel_crtc->border_size)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
@@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
 	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
 }
 
-static inline u32 panel_fitter_scaling(u32 source, u32 target)
-{
-	/*
-	 * Floating point operation is not supported. So the FACTOR
-	 * is defined, which can avoid the floating point computation
-	 * when calculating the panel ratio.
-	 */
-#define ACCURACY 12
-#define FACTOR (1 << ACCURACY)
-	u32 ratio = source * FACTOR / target;
-	return (FACTOR * ratio + FACTOR/2) / FACTOR;
-}
-
 static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
 			      u32 *pfit_control)
 {
@@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	if (dst_width != tot_width)
+		centre_horizontally(adjusted_mode, dst_width);
+	if (dst_height != tot_height)
+		centre_vertically(adjusted_mode, dst_height);
+
+	/* No scaling needed now, but still enable the panel fitter,
+	   as that will allow the User to subequently do the dynamic
+	   flipping of fbs of different resolutions */
+	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
+		BUG_ON(!intel_crtc->pfit_enabled);
+		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
+	}
+
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if User wants to apply the borders, or wants to forcefully
+	   enable the panel fitter, otherwise fall through the regular path */
+	if (intel_crtc->pfit_enabled ||
+			intel_crtc->border_size)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-26  3:55         ` [PATCH v4 3/3] " akash.goel
@ 2014-04-07  4:06           ` Goel, Akash
  2014-04-08 16:31             ` Damien Lespiau
  2014-04-08 16:28           ` Ville Syrjälä
  1 sibling, 1 reply; 26+ messages in thread
From: Goel, Akash @ 2014-04-07  4:06 UTC (permalink / raw)
  To: Ville Syrjälä, Daniel Vetter; +Cc: intel-gfx

Hi Ville,

Please could you review this patch.

Best Regards
Akash

-----Original Message-----
From: Goel, Akash 
Sent: Wednesday, March 26, 2014 9:25 AM
To: intel-gfx@lists.freedesktop.org
Cc: Purushothaman, Vijay A; G, Pallavi; Goel, Akash
Subject: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

v2: Added a new check for the invalid border size input

v3: Fixed bugs in output window calculation Removed superfluous checks

v4: Added the capability to forecfully enable the Panel fitter.
The property value is of 64 bits, first 32 bits are used for border dimensions. The 33rd bit can be used to forcefully enable the panel fitter. This is useful for Panels which do not override the User specified Pipe timings.

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 ++
 drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   5 +
 drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
 4 files changed, 211 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6f3af15..eec32ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Property to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *output_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7149123..a217f25 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10447,7 +10447,23 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
+
+	if (property == dev_priv->output_border_property) {
+		if ((val == (uint64_t)intel_crtc->border_size) &&
+		    (((val >> 32) & 0x1) == intel_crtc->pfit_enabled))
+			return 0;
+
+		intel_crtc->border_size = (uint32_t)val;
+		intel_crtc->pfit_enabled = (val >> 32) & 0x1;
+		if ((intel_crtc->border_size != 0) &&
+		    (!intel_crtc->pfit_enabled)) {
+			DRM_ERROR("Wrong setting, expect Pfit to be enabled when "
+				  "applying borders\n");
+			return -EINVAL;
+		}
+
+		goto done;
+	}
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff); @@ -10488,7 +10504,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
+
+done:
+	if (crtc)
+		intel_crtc_restore_mode(crtc);
+
+	return 0;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = { @@ -10641,12 +10663,23 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	if (!dev_priv->input_size_property)
 		dev_priv->input_size_property =
-			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
+			drm_property_create_range(dev, 0, "input size",
+						0, 0xFFFFFFFF);
 
 	if (dev_priv->input_size_property)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (!dev_priv->output_border_property)
+		dev_priv->output_border_property =
+			drm_property_create_range(dev, 0, "border size",
+						0, (uint64_t)0x1FFFFFFFF);
+
+	if (dev_priv->output_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->output_border_property,
+					   0);
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5360d16..3cfc9da 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,11 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* for forceful enabling of panel fitter */
+	bool pfit_enabled;
+	/* border info for the output/display window */
+	uint32_t border_size;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */ diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index cb05840..86a486e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -29,10 +29,25 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format 
+*/ #define MAX_DOWNSCALE_RATIO  (0x9 << 9)
 
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
+static inline u32 panel_fitter_scaling(u32 source, u32 target) {
+	/*
+	 * Floating point operation is not supported. So the FACTOR
+	 * is defined, which can avoid the floating point computation
+	 * when calculating the panel ratio.
+	 */
+#define ACCURACY 12
+#define FACTOR (1 << ACCURACY)
+	u32 ratio = source * FACTOR / target;
+	return (FACTOR * ratio + FACTOR/2) / FACTOR; }
+
 void
 intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode) @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);  }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config) {
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width  = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
+	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0; }
+
 /* adjusted_mode has been preset to be the panel's fixed mode */  void  intel_pch_panel_fitting(struct intel_crtc *intel_crtc, @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if User wants to apply the borders, or wants to forcefully
+	   enable the panel fitter, otherwise fall through the regular path */
+	if (intel_crtc->pfit_enabled ||
+			intel_crtc->border_size)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h) @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
 	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;  }
 
-static inline u32 panel_fitter_scaling(u32 source, u32 target) -{
-	/*
-	 * Floating point operation is not supported. So the FACTOR
-	 * is defined, which can avoid the floating point computation
-	 * when calculating the panel ratio.
-	 */
-#define ACCURACY 12
-#define FACTOR (1 << ACCURACY)
-	u32 ratio = source * FACTOR / target;
-	return (FACTOR * ratio + FACTOR/2) / FACTOR;
-}
-
 static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
 			      u32 *pfit_control)
 {
@@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config) {
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width -
+			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
+	dst_height = tot_height -
+			((intel_crtc->border_size & 0xffff) * 2);
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	if (dst_width != tot_width)
+		centre_horizontally(adjusted_mode, dst_width);
+	if (dst_height != tot_height)
+		centre_vertically(adjusted_mode, dst_height);
+
+	/* No scaling needed now, but still enable the panel fitter,
+	   as that will allow the User to subequently do the dynamic
+	   flipping of fbs of different resolutions */
+	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
+		BUG_ON(!intel_crtc->pfit_enabled);
+		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
+	}
+
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border; }
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if User wants to apply the borders, or wants to forcefully
+	   enable the panel fitter, otherwise fall through the regular path */
+	if (intel_crtc->pfit_enabled ||
+			intel_crtc->border_size)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
--
1.8.5.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-03-26  3:55         ` [PATCH v4 3/3] " akash.goel
  2014-04-07  4:06           ` Goel, Akash
@ 2014-04-08 16:28           ` Ville Syrjälä
  2014-04-10  7:43             ` Akash Goel
  1 sibling, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2014-04-08 16:28 UTC (permalink / raw)
  To: akash.goel; +Cc: intel-gfx

On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> This patch adds a new drm crtc property for varying the size of
> the horizontal & vertical borers of the output/display window.
> This will control the output of Panel fitter.
> 
> v2: Added a new check for the invalid border size input
> 
> v3: Fixed bugs in output window calculation
> Removed superfluous checks
> 
> v4: Added the capability to forecfully enable the Panel fitter.
> The property value is of 64 bits, first 32 bits are used for
> border dimensions. The 33rd bit can be used to forcefully
> enable the panel fitter. This is useful for Panels which
> do not override the User specified Pipe timings.
> 
> Testcase: igt/kms_panel_fitter_test
> 
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
>  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |   5 +
>  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
>  4 files changed, 211 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6f3af15..eec32ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
>  	 */
>  	struct drm_property *input_size_property;
>  
> +	/*
> +	 * Property to dynamically vary the size of the
> +	 * borders. This will indirectly control the size
> +	 * of the display window i.e Panel fitter output
> +	 */
> +	struct drm_property *output_border_property;
> +
>  	uint32_t hw_context_size;
>  	struct list_head context_list;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7149123..a217f25 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10447,7 +10447,23 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int ret = -ENOENT;
> +
> +	if (property == dev_priv->output_border_property) {
> +		if ((val == (uint64_t)intel_crtc->border_size) &&
> +		    (((val >> 32) & 0x1) == intel_crtc->pfit_enabled))
> +			return 0;
> +
> +		intel_crtc->border_size = (uint32_t)val;
> +		intel_crtc->pfit_enabled = (val >> 32) & 0x1;
> +		if ((intel_crtc->border_size != 0) &&
> +		    (!intel_crtc->pfit_enabled)) {
> +			DRM_ERROR("Wrong setting, expect Pfit to be enabled when "
> +				  "applying borders\n");
> +			return -EINVAL;
> +		}
> +
> +		goto done;
> +	}
>  
>  	if (property == dev_priv->input_size_property) {
>  		int new_width = (int)((val >> 16) & 0xffff);
> @@ -10488,7 +10504,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
>  		return 0;
>  	}
>  
> -	return ret;
> +	return -EINVAL;
> +
> +done:
> +	if (crtc)
> +		intel_crtc_restore_mode(crtc);
> +
> +	return 0;
>  }
>  
>  static const struct drm_crtc_funcs intel_crtc_funcs = {
> @@ -10641,12 +10663,23 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	if (!dev_priv->input_size_property)
>  		dev_priv->input_size_property =
> -			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
> +			drm_property_create_range(dev, 0, "input size",
> +						0, 0xFFFFFFFF);
>  
>  	if (dev_priv->input_size_property)
>  		drm_object_attach_property(&intel_crtc->base.base,
>  					   dev_priv->input_size_property,
>  					   0);
> +
> +	if (!dev_priv->output_border_property)
> +		dev_priv->output_border_property =
> +			drm_property_create_range(dev, 0, "border size",
> +						0, (uint64_t)0x1FFFFFFFF);
> +
> +	if (dev_priv->output_border_property)
> +		drm_object_attach_property(&intel_crtc->base.base,
> +					   dev_priv->output_border_property,
> +					   0);
>  }
>  
>  enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5360d16..3cfc9da 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -387,6 +387,11 @@ struct intel_crtc {
>  	bool cpu_fifo_underrun_disabled;
>  	bool pch_fifo_underrun_disabled;
>  
> +	/* for forceful enabling of panel fitter */
> +	bool pfit_enabled;
> +	/* border info for the output/display window */
> +	uint32_t border_size;
> +
>  	/* per-pipe watermark state */
>  	struct {
>  		/* watermarks currently being used  */
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index cb05840..86a486e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -29,10 +29,25 @@
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
> +#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
>  
>  #include <linux/moduleparam.h>
>  #include "intel_drv.h"
>  
> +static inline u32 panel_fitter_scaling(u32 source, u32 target)
> +{
> +	/*
> +	 * Floating point operation is not supported. So the FACTOR
> +	 * is defined, which can avoid the floating point computation
> +	 * when calculating the panel ratio.
> +	 */
> +#define ACCURACY 12
> +#define FACTOR (1 << ACCURACY)
> +	u32 ratio = source * FACTOR / target;
> +	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> +}
> +
>  void
>  intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  		       struct drm_display_mode *adjusted_mode)
> @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  	drm_mode_set_crtcinfo(adjusted_mode, 0);
>  }
>  
> +void
> +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> +			struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_display_mode *adjusted_mode;
> +	int x, y;
> +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> +	u32 tot_width, tot_height;
> +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> +	u32 dst_width, dst_height;
> +
> +	adjusted_mode = &pipe_config->adjusted_mode;
> +
> +	src_width = pipe_config->pipe_src_w;
> +	src_height = pipe_config->pipe_src_h;
> +
> +	tot_width  = adjusted_mode->hdisplay;
> +	tot_height = adjusted_mode->vdisplay;
> +
> +	/*
> +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> +	 * region. So (HACTIVE - Left border - Right Border) *
> +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> +	 * output rectangle on screen
> +	 */
> +	dst_width = tot_width -
> +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> +	dst_height = tot_height -
> +			((intel_crtc->border_size & 0xffff) * 2);

I'm thinking that we should allow the user to specify each border width
individually rather than just assume left==right and top==bottom.

> +
> +	if ((dst_width == 0) || (dst_height == 0)) {
> +		DRM_ERROR("Invalid border size input\n");
> +		return;

This is clear sign here that we should do all this stuff in the compute
config stage so that we can fail gracefully and tell userspace that things
didn't work out.

> +	}
> +
> +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> +
> +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> +		DRM_ERROR("width is too small\n");
> +		return;
> +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> +		DRM_ERROR("height is too small\n");
> +		return;
> +	}
> +
> +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> +
> +	pipe_config->pch_pfit.pos = (x << 16) | y;
> +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> +}
> +
>  /* adjusted_mode has been preset to be the panel's fixed mode */
>  void
>  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
>  
>  	x = y = width = height = 0;
>  
> +	/* check if User wants to apply the borders, or wants to forcefully
> +	   enable the panel fitter, otherwise fall through the regular path */
> +	if (intel_crtc->pfit_enabled ||
> +			intel_crtc->border_size)
> +		return intel_pch_manual_panel_fitting(intel_crtc,
> +						      pipe_config);
> +
>  	/* Native modes don't need fitting */
>  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
>  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
>  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
>  }
>  
> -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> -{
> -	/*
> -	 * Floating point operation is not supported. So the FACTOR
> -	 * is defined, which can avoid the floating point computation
> -	 * when calculating the panel ratio.
> -	 */
> -#define ACCURACY 12
> -#define FACTOR (1 << ACCURACY)
> -	u32 ratio = source * FACTOR / target;
> -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> -}
> -
>  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
>  			      u32 *pfit_control)
>  {
> @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
>  	}
>  }
>  
> +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> +				     struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = intel_crtc->base.dev;
> +	u32 pfit_control = 0, border = 0;
> +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> +	struct drm_display_mode *adjusted_mode;
> +	u32 tot_width, tot_height;
> +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> +	u32 dst_width, dst_height;
> +
> +	adjusted_mode = &pipe_config->adjusted_mode;
> +
> +	src_width = pipe_config->pipe_src_w;
> +	src_height = pipe_config->pipe_src_h;
> +
> +	tot_width = adjusted_mode->hdisplay;
> +	tot_height = adjusted_mode->vdisplay;
> +
> +	/*
> +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> +	 * region. So (HACTIVE - Left border - Right Border) *
> +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> +	 * output rectangle on screen
> +	 */
> +	dst_width = tot_width -
> +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> +	dst_height = tot_height -
> +			((intel_crtc->border_size & 0xffff) * 2);
> +
> +	if ((dst_width == 0) || (dst_height == 0)) {
> +		DRM_ERROR("Invalid border size input\n");
> +		return;
> +	}
> +
> +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> +
> +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> +		DRM_ERROR("width is too small\n");
> +		return;
> +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> +		DRM_ERROR("height is too small\n");
> +		return;
> +	}
> +
> +	if (dst_width != tot_width)
> +		centre_horizontally(adjusted_mode, dst_width);
> +	if (dst_height != tot_height)
> +		centre_vertically(adjusted_mode, dst_height);
> +
> +	/* No scaling needed now, but still enable the panel fitter,
> +	   as that will allow the User to subequently do the dynamic
> +	   flipping of fbs of different resolutions */
> +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> +		BUG_ON(!intel_crtc->pfit_enabled);
> +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> +	}
> +
> +	border = LVDS_BORDER_ENABLE;
> +
> +	if (INTEL_INFO(dev)->gen >= 4) {
> +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> +	} else {
> +		pfit_control |= (PFIT_ENABLE |
> +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> +	}
> +
> +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> +
> +	pipe_config->gmch_pfit.control = pfit_control;
> +	pipe_config->gmch_pfit.lvds_border_bits = border;
> +}
> +
>  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  			      struct intel_crtc_config *pipe_config,
>  			      int fitting_mode)
> @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  
>  	adjusted_mode = &pipe_config->adjusted_mode;
>  
> +	/* check if User wants to apply the borders, or wants to forcefully
> +	   enable the panel fitter, otherwise fall through the regular path */
> +	if (intel_crtc->pfit_enabled ||
> +			intel_crtc->border_size)
> +		return intel_gmch_manual_panel_fitting(intel_crtc,
> +						       pipe_config);
> +
>  	/* Native modes don't need fitting */
>  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
>  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> -- 
> 1.8.5.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-07  4:06           ` Goel, Akash
@ 2014-04-08 16:31             ` Damien Lespiau
  2014-04-08 16:32               ` Damien Lespiau
  0 siblings, 1 reply; 26+ messages in thread
From: Damien Lespiau @ 2014-04-08 16:31 UTC (permalink / raw)
  To: Goel, Akash; +Cc: intel-gfx

On Mon, Apr 07, 2014 at 04:06:34AM +0000, Goel, Akash wrote:
> Hi Ville,
> 
> Please could you review this patch.

You need a pass of checkpatch.pl in here. Not sure what happened with
the coding style.

-- 
Damien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-08 16:31             ` Damien Lespiau
@ 2014-04-08 16:32               ` Damien Lespiau
  0 siblings, 0 replies; 26+ messages in thread
From: Damien Lespiau @ 2014-04-08 16:32 UTC (permalink / raw)
  To: Goel, Akash; +Cc: intel-gfx

On Tue, Apr 08, 2014 at 05:31:06PM +0100, Damien Lespiau wrote:
> On Mon, Apr 07, 2014 at 04:06:34AM +0000, Goel, Akash wrote:
> > Hi Ville,
> > 
> > Please could you review this patch.
> 
> You need a pass of checkpatch.pl in here. Not sure what happened with
> the coding style.

Ignore this, I was looking at the wrong thing.

-- 
Damien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-08 16:28           ` Ville Syrjälä
@ 2014-04-10  7:43             ` Akash Goel
  2014-04-10 11:34               ` Ville Syrjälä
  0 siblings, 1 reply; 26+ messages in thread
From: Akash Goel @ 2014-04-10  7:43 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
> On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> > From: Akash Goel <akash.goel@intel.com>
> > 
> > This patch adds a new drm crtc property for varying the size of
> > the horizontal & vertical borers of the output/display window.
> > This will control the output of Panel fitter.
> > 
> > v2: Added a new check for the invalid border size input
> > 
> > v3: Fixed bugs in output window calculation
> > Removed superfluous checks
> > 
> > v4: Added the capability to forecfully enable the Panel fitter.
> > The property value is of 64 bits, first 32 bits are used for
> > border dimensions. The 33rd bit can be used to forcefully
> > enable the panel fitter. This is useful for Panels which
> > do not override the User specified Pipe timings.
> > 
> > Testcase: igt/kms_panel_fitter_test
> > 
> > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
> >  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
> >  drivers/gpu/drm/i915/intel_drv.h     |   5 +
> >  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
> >  4 files changed, 211 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 6f3af15..eec32ed 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1614,6 +1614,13 @@ typedef struct drm_i915_private {
> >  	 */
> >  	struct drm_property *input_size_property;
> >  
> > +	/*
> > +	 * Property to dynamically vary the size of the
> > +	 * borders. This will indirectly control the size
> > +	 * of the display window i.e Panel fitter output
> > +	 */
> > +	struct drm_property *output_border_property;
> > +
> >  	uint32_t hw_context_size;
> >  	struct list_head context_list;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7149123..a217f25 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10447,7 +10447,23 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -	int ret = -ENOENT;
> > +
> > +	if (property == dev_priv->output_border_property) {
> > +		if ((val == (uint64_t)intel_crtc->border_size) &&
> > +		    (((val >> 32) & 0x1) == intel_crtc->pfit_enabled))
> > +			return 0;
> > +
> > +		intel_crtc->border_size = (uint32_t)val;
> > +		intel_crtc->pfit_enabled = (val >> 32) & 0x1;
> > +		if ((intel_crtc->border_size != 0) &&
> > +		    (!intel_crtc->pfit_enabled)) {
> > +			DRM_ERROR("Wrong setting, expect Pfit to be enabled when "
> > +				  "applying borders\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		goto done;
> > +	}
> >  
> >  	if (property == dev_priv->input_size_property) {
> >  		int new_width = (int)((val >> 16) & 0xffff);
> > @@ -10488,7 +10504,13 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
> >  		return 0;
> >  	}
> >  
> > -	return ret;
> > +	return -EINVAL;
> > +
> > +done:
> > +	if (crtc)
> > +		intel_crtc_restore_mode(crtc);
> > +
> > +	return 0;
> >  }
> >  
> >  static const struct drm_crtc_funcs intel_crtc_funcs = {
> > @@ -10641,12 +10663,23 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  
> >  	if (!dev_priv->input_size_property)
> >  		dev_priv->input_size_property =
> > -			drm_property_create_range(dev, 0, "input size", 0, 0xFFFFFFFF);
> > +			drm_property_create_range(dev, 0, "input size",
> > +						0, 0xFFFFFFFF);
> >  
> >  	if (dev_priv->input_size_property)
> >  		drm_object_attach_property(&intel_crtc->base.base,
> >  					   dev_priv->input_size_property,
> >  					   0);
> > +
> > +	if (!dev_priv->output_border_property)
> > +		dev_priv->output_border_property =
> > +			drm_property_create_range(dev, 0, "border size",
> > +						0, (uint64_t)0x1FFFFFFFF);
> > +
> > +	if (dev_priv->output_border_property)
> > +		drm_object_attach_property(&intel_crtc->base.base,
> > +					   dev_priv->output_border_property,
> > +					   0);
> >  }
> >  
> >  enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5360d16..3cfc9da 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -387,6 +387,11 @@ struct intel_crtc {
> >  	bool cpu_fifo_underrun_disabled;
> >  	bool pch_fifo_underrun_disabled;
> >  
> > +	/* for forceful enabling of panel fitter */
> > +	bool pfit_enabled;
> > +	/* border info for the output/display window */
> > +	uint32_t border_size;
> > +
> >  	/* per-pipe watermark state */
> >  	struct {
> >  		/* watermarks currently being used  */
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index cb05840..86a486e 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -29,10 +29,25 @@
> >   */
> >  
> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
> > +#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
> >  
> >  #include <linux/moduleparam.h>
> >  #include "intel_drv.h"
> >  
> > +static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > +{
> > +	/*
> > +	 * Floating point operation is not supported. So the FACTOR
> > +	 * is defined, which can avoid the floating point computation
> > +	 * when calculating the panel ratio.
> > +	 */
> > +#define ACCURACY 12
> > +#define FACTOR (1 << ACCURACY)
> > +	u32 ratio = source * FACTOR / target;
> > +	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > +}
> > +
> >  void
> >  intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> >  		       struct drm_display_mode *adjusted_mode)
> > @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> >  }
> >  
> > +void
> > +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > +			struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_display_mode *adjusted_mode;
> > +	int x, y;
> > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > +	u32 tot_width, tot_height;
> > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > +	u32 dst_width, dst_height;
> > +
> > +	adjusted_mode = &pipe_config->adjusted_mode;
> > +
> > +	src_width = pipe_config->pipe_src_w;
> > +	src_height = pipe_config->pipe_src_h;
> > +
> > +	tot_width  = adjusted_mode->hdisplay;
> > +	tot_height = adjusted_mode->vdisplay;
> > +
> > +	/*
> > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > +	 * region. So (HACTIVE - Left border - Right Border) *
> > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > +	 * output rectangle on screen
> > +	 */
> > +	dst_width = tot_width -
> > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > +	dst_height = tot_height -
> > +			((intel_crtc->border_size & 0xffff) * 2);
> 
> I'm thinking that we should allow the user to specify each border width
> individually rather than just assume left==right and top==bottom.
> 

Sorry I thought that the Top/Bottom & left/Right borders would be
symmetric only.
Tried setting the borders on EDP & HDMI panels by manipulating the Pipe
timings (through the logic used in 'centre_horizontally' &
'centre_vertically' functions), but it didn't work.
Is this logic effective for the LVDS panel only ?

> > +
> > +	if ((dst_width == 0) || (dst_height == 0)) {
> > +		DRM_ERROR("Invalid border size input\n");
> > +		return;
> 
> This is clear sign here that we should do all this stuff in the compute
> config stage so that we can fail gracefully and tell userspace that things
> didn't work out.
> 

Actually this call to decide the panel fitter config, is being made in
the context of 'compute config' only. But it's originating from the
'crtc set property' & not from the modeset.
Do you mean to say that if done from the 'modeset' call, we can report
back the error to User space for an invalid border setting.
Actually currently the return type is 'void' only for the 2 existing
panel fitter config functions. 
Also we don't have a check in place for the max supported upscaling
ratio.

> > +	}
> > +
> > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > +
> > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > +		DRM_ERROR("width is too small\n");
> > +		return;
> > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > +		DRM_ERROR("height is too small\n");
> > +		return;
> > +	}
> > +
> > +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> > +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> > +
> > +	pipe_config->pch_pfit.pos = (x << 16) | y;
> > +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> > +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> > +}
> > +
> >  /* adjusted_mode has been preset to be the panel's fixed mode */
> >  void
> >  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> >  
> >  	x = y = width = height = 0;
> >  
> > +	/* check if User wants to apply the borders, or wants to forcefully
> > +	   enable the panel fitter, otherwise fall through the regular path */
> > +	if (intel_crtc->pfit_enabled ||
> > +			intel_crtc->border_size)
> > +		return intel_pch_manual_panel_fitting(intel_crtc,
> > +						      pipe_config);
> > +
> >  	/* Native modes don't need fitting */
> >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
> >  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
> >  }
> >  
> > -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > -{
> > -	/*
> > -	 * Floating point operation is not supported. So the FACTOR
> > -	 * is defined, which can avoid the floating point computation
> > -	 * when calculating the panel ratio.
> > -	 */
> > -#define ACCURACY 12
> > -#define FACTOR (1 << ACCURACY)
> > -	u32 ratio = source * FACTOR / target;
> > -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > -}
> > -
> >  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> >  			      u32 *pfit_control)
> >  {
> > @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> >  	}
> >  }
> >  
> > +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > +				     struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_device *dev = intel_crtc->base.dev;
> > +	u32 pfit_control = 0, border = 0;
> > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > +	struct drm_display_mode *adjusted_mode;
> > +	u32 tot_width, tot_height;
> > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > +	u32 dst_width, dst_height;
> > +
> > +	adjusted_mode = &pipe_config->adjusted_mode;
> > +
> > +	src_width = pipe_config->pipe_src_w;
> > +	src_height = pipe_config->pipe_src_h;
> > +
> > +	tot_width = adjusted_mode->hdisplay;
> > +	tot_height = adjusted_mode->vdisplay;
> > +
> > +	/*
> > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > +	 * region. So (HACTIVE - Left border - Right Border) *
> > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > +	 * output rectangle on screen
> > +	 */
> > +	dst_width = tot_width -
> > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > +	dst_height = tot_height -
> > +			((intel_crtc->border_size & 0xffff) * 2);
> > +
> > +	if ((dst_width == 0) || (dst_height == 0)) {
> > +		DRM_ERROR("Invalid border size input\n");
> > +		return;
> > +	}
> > +
> > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > +
> > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > +		DRM_ERROR("width is too small\n");
> > +		return;
> > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > +		DRM_ERROR("height is too small\n");
> > +		return;
> > +	}
> > +
> > +	if (dst_width != tot_width)
> > +		centre_horizontally(adjusted_mode, dst_width);
> > +	if (dst_height != tot_height)
> > +		centre_vertically(adjusted_mode, dst_height);
> > +
> > +	/* No scaling needed now, but still enable the panel fitter,
> > +	   as that will allow the User to subequently do the dynamic
> > +	   flipping of fbs of different resolutions */
> > +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> > +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> > +		BUG_ON(!intel_crtc->pfit_enabled);
> > +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> > +	}
> > +
> > +	border = LVDS_BORDER_ENABLE;
> > +
> > +	if (INTEL_INFO(dev)->gen >= 4) {
> > +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> > +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> > +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> > +	} else {
> > +		pfit_control |= (PFIT_ENABLE |
> > +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> > +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> > +	}
> > +
> > +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> > +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> > +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > +
> > +	pipe_config->gmch_pfit.control = pfit_control;
> > +	pipe_config->gmch_pfit.lvds_border_bits = border;
> > +}
> > +
> >  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> >  			      struct intel_crtc_config *pipe_config,
> >  			      int fitting_mode)
> > @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> >  
> >  	adjusted_mode = &pipe_config->adjusted_mode;
> >  
> > +	/* check if User wants to apply the borders, or wants to forcefully
> > +	   enable the panel fitter, otherwise fall through the regular path */
> > +	if (intel_crtc->pfit_enabled ||
> > +			intel_crtc->border_size)
> > +		return intel_gmch_manual_panel_fitting(intel_crtc,
> > +						       pipe_config);
> > +
> >  	/* Native modes don't need fitting */
> >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > -- 
> > 1.8.5.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-10  7:43             ` Akash Goel
@ 2014-04-10 11:34               ` Ville Syrjälä
  2014-04-11  3:04                 ` Akash Goel
  0 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2014-04-10 11:34 UTC (permalink / raw)
  To: Akash Goel; +Cc: intel-gfx

On Thu, Apr 10, 2014 at 01:13:56PM +0530, Akash Goel wrote:
> On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
> > On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> > > From: Akash Goel <akash.goel@intel.com>
> > > 
> > > This patch adds a new drm crtc property for varying the size of
> > > the horizontal & vertical borers of the output/display window.
> > > This will control the output of Panel fitter.
> > > 
> > > v2: Added a new check for the invalid border size input
> > > 
> > > v3: Fixed bugs in output window calculation
> > > Removed superfluous checks
> > > 
> > > v4: Added the capability to forecfully enable the Panel fitter.
> > > The property value is of 64 bits, first 32 bits are used for
> > > border dimensions. The 33rd bit can be used to forcefully
> > > enable the panel fitter. This is useful for Panels which
> > > do not override the User specified Pipe timings.
> > > 
> > > Testcase: igt/kms_panel_fitter_test
> > > 
> > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
> > >  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
> > >  drivers/gpu/drm/i915/intel_drv.h     |   5 +
> > >  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
> > >  4 files changed, 211 insertions(+), 16 deletions(-)
> > > 
<snip>
> > > @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> > >  }
> > >  
> > > +void
> > > +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > +			struct intel_crtc_config *pipe_config)
> > > +{
> > > +	struct drm_display_mode *adjusted_mode;
> > > +	int x, y;
> > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > +	u32 tot_width, tot_height;
> > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > +	u32 dst_width, dst_height;
> > > +
> > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > +
> > > +	src_width = pipe_config->pipe_src_w;
> > > +	src_height = pipe_config->pipe_src_h;
> > > +
> > > +	tot_width  = adjusted_mode->hdisplay;
> > > +	tot_height = adjusted_mode->vdisplay;
> > > +
> > > +	/*
> > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > +	 * output rectangle on screen
> > > +	 */
> > > +	dst_width = tot_width -
> > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > +	dst_height = tot_height -
> > > +			((intel_crtc->border_size & 0xffff) * 2);
> > 
> > I'm thinking that we should allow the user to specify each border width
> > individually rather than just assume left==right and top==bottom.
> > 
> 
> Sorry I thought that the Top/Bottom & left/Right borders would be
> symmetric only.

I don't see a reason to limit ourselves here.

> Tried setting the borders on EDP & HDMI panels by manipulating the Pipe
> timings (through the logic used in 'centre_horizontally' &
> 'centre_vertically' functions), but it didn't work.
> Is this logic effective for the LVDS panel only ?

Could be. Certainly the border enable bit is there only for LVDS. The
gmch panel fitter isn't very flexible so it's possible we can't
actually make it do many of the things the pch pfit can do.

What happens if we set up the pfit to use manual scaling ratios but
configure both scaling ratios so that scaled image won't fill the
active video region in either direction? Does it position the scaled
image at coordinates 0,0 and simply scan black the rest of the time after
it's run out of source pixel data? Or does it automagically center the
image and scan black on both sides? Or does it fail in some way?

> 
> > > +
> > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > +		DRM_ERROR("Invalid border size input\n");
> > > +		return;
> > 
> > This is clear sign here that we should do all this stuff in the compute
> > config stage so that we can fail gracefully and tell userspace that things
> > didn't work out.
> > 
> 
> Actually this call to decide the panel fitter config, is being made in
> the context of 'compute config' only. But it's originating from the
> 'crtc set property' & not from the modeset.

We need to check things in both cases, and return an error if things
can't work out.

> Do you mean to say that if done from the 'modeset' call, we can report
> back the error to User space for an invalid border setting.
> Actually currently the return type is 'void' only for the 2 existing
> panel fitter config functions. 
> Also we don't have a check in place for the max supported upscaling
> ratio.

Is there an upscaling limit? I know there's a downscaling limit of IIRC
1.125x or something close to that. I don't think we check that either.

> 
> > > +	}
> > > +
> > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > +
> > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > +		DRM_ERROR("width is too small\n");
> > > +		return;
> > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > +		DRM_ERROR("height is too small\n");
> > > +		return;
> > > +	}
> > > +
> > > +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> > > +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> > > +
> > > +	pipe_config->pch_pfit.pos = (x << 16) | y;
> > > +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> > > +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> > > +}
> > > +
> > >  /* adjusted_mode has been preset to be the panel's fixed mode */
> > >  void
> > >  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > >  
> > >  	x = y = width = height = 0;
> > >  
> > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > +	if (intel_crtc->pfit_enabled ||
> > > +			intel_crtc->border_size)
> > > +		return intel_pch_manual_panel_fitting(intel_crtc,
> > > +						      pipe_config);
> > > +
> > >  	/* Native modes don't need fitting */
> > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
> > >  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
> > >  }
> > >  
> > > -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > > -{
> > > -	/*
> > > -	 * Floating point operation is not supported. So the FACTOR
> > > -	 * is defined, which can avoid the floating point computation
> > > -	 * when calculating the panel ratio.
> > > -	 */
> > > -#define ACCURACY 12
> > > -#define FACTOR (1 << ACCURACY)
> > > -	u32 ratio = source * FACTOR / target;
> > > -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > > -}
> > > -
> > >  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> > >  			      u32 *pfit_control)
> > >  {
> > > @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> > >  	}
> > >  }
> > >  
> > > +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > +				     struct intel_crtc_config *pipe_config)
> > > +{
> > > +	struct drm_device *dev = intel_crtc->base.dev;
> > > +	u32 pfit_control = 0, border = 0;
> > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > +	struct drm_display_mode *adjusted_mode;
> > > +	u32 tot_width, tot_height;
> > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > +	u32 dst_width, dst_height;
> > > +
> > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > +
> > > +	src_width = pipe_config->pipe_src_w;
> > > +	src_height = pipe_config->pipe_src_h;
> > > +
> > > +	tot_width = adjusted_mode->hdisplay;
> > > +	tot_height = adjusted_mode->vdisplay;
> > > +
> > > +	/*
> > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > +	 * output rectangle on screen
> > > +	 */
> > > +	dst_width = tot_width -
> > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > +	dst_height = tot_height -
> > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > +
> > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > +		DRM_ERROR("Invalid border size input\n");
> > > +		return;
> > > +	}
> > > +
> > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > +
> > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > +		DRM_ERROR("width is too small\n");
> > > +		return;
> > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > +		DRM_ERROR("height is too small\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (dst_width != tot_width)
> > > +		centre_horizontally(adjusted_mode, dst_width);
> > > +	if (dst_height != tot_height)
> > > +		centre_vertically(adjusted_mode, dst_height);
> > > +
> > > +	/* No scaling needed now, but still enable the panel fitter,
> > > +	   as that will allow the User to subequently do the dynamic
> > > +	   flipping of fbs of different resolutions */
> > > +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> > > +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> > > +		BUG_ON(!intel_crtc->pfit_enabled);
> > > +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> > > +	}
> > > +
> > > +	border = LVDS_BORDER_ENABLE;
> > > +
> > > +	if (INTEL_INFO(dev)->gen >= 4) {
> > > +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> > > +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> > > +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> > > +	} else {
> > > +		pfit_control |= (PFIT_ENABLE |
> > > +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> > > +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> > > +	}
> > > +
> > > +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> > > +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> > > +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > > +
> > > +	pipe_config->gmch_pfit.control = pfit_control;
> > > +	pipe_config->gmch_pfit.lvds_border_bits = border;
> > > +}
> > > +
> > >  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > >  			      struct intel_crtc_config *pipe_config,
> > >  			      int fitting_mode)
> > > @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > >  
> > >  	adjusted_mode = &pipe_config->adjusted_mode;
> > >  
> > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > +	if (intel_crtc->pfit_enabled ||
> > > +			intel_crtc->border_size)
> > > +		return intel_gmch_manual_panel_fitting(intel_crtc,
> > > +						       pipe_config);
> > > +
> > >  	/* Native modes don't need fitting */
> > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > -- 
> > > 1.8.5.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-10 11:34               ` Ville Syrjälä
@ 2014-04-11  3:04                 ` Akash Goel
  2014-04-11 11:42                   ` Ville Syrjälä
  0 siblings, 1 reply; 26+ messages in thread
From: Akash Goel @ 2014-04-11  3:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, 2014-04-10 at 14:34 +0300, Ville Syrjälä wrote:
> On Thu, Apr 10, 2014 at 01:13:56PM +0530, Akash Goel wrote:
> > On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
> > > On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> > > > From: Akash Goel <akash.goel@intel.com>
> > > > 
> > > > This patch adds a new drm crtc property for varying the size of
> > > > the horizontal & vertical borers of the output/display window.
> > > > This will control the output of Panel fitter.
> > > > 
> > > > v2: Added a new check for the invalid border size input
> > > > 
> > > > v3: Fixed bugs in output window calculation
> > > > Removed superfluous checks
> > > > 
> > > > v4: Added the capability to forecfully enable the Panel fitter.
> > > > The property value is of 64 bits, first 32 bits are used for
> > > > border dimensions. The 33rd bit can be used to forcefully
> > > > enable the panel fitter. This is useful for Panels which
> > > > do not override the User specified Pipe timings.
> > > > 
> > > > Testcase: igt/kms_panel_fitter_test
> > > > 
> > > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
> > > >  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
> > > >  drivers/gpu/drm/i915/intel_drv.h     |   5 +
> > > >  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
> > > >  4 files changed, 211 insertions(+), 16 deletions(-)
> > > > 
> <snip>
> > > > @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > > >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> > > >  }
> > > >  
> > > > +void
> > > > +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > +			struct intel_crtc_config *pipe_config)
> > > > +{
> > > > +	struct drm_display_mode *adjusted_mode;
> > > > +	int x, y;
> > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > +	u32 tot_width, tot_height;
> > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > +	u32 dst_width, dst_height;
> > > > +
> > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > +
> > > > +	src_width = pipe_config->pipe_src_w;
> > > > +	src_height = pipe_config->pipe_src_h;
> > > > +
> > > > +	tot_width  = adjusted_mode->hdisplay;
> > > > +	tot_height = adjusted_mode->vdisplay;
> > > > +
> > > > +	/*
> > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > +	 * output rectangle on screen
> > > > +	 */
> > > > +	dst_width = tot_width -
> > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > +	dst_height = tot_height -
> > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > 
> > > I'm thinking that we should allow the user to specify each border width
> > > individually rather than just assume left==right and top==bottom.
> > > 
> > 
> > Sorry I thought that the Top/Bottom & left/Right borders would be
> > symmetric only.
> 
> I don't see a reason to limit ourselves here.
> 

Fine, will extend this property to set each border separately. 
Can I use the 12 bits for each border value, as that shall be
sufficient.

> > Tried setting the borders on EDP & HDMI panels by manipulating the Pipe
> > timings (through the logic used in 'centre_horizontally' &
> > 'centre_vertically' functions), but it didn't work.
> > Is this logic effective for the LVDS panel only ?
> 
> Could be. Certainly the border enable bit is there only for LVDS. The
> gmch panel fitter isn't very flexible so it's possible we can't
> actually make it do many of the things the pch pfit can do.
> 

Yes the GMCH panel fitter function is not equally capable as the PCH
counterpart. Here except the LVDS panel, it seems that borders cannot be
realized on any other panel, just via the manipulation of Pipe timings
(the way it can be done in PCH one).
For the same reason the 'Center' Panel fitting mode of "scaling mode"
property is not working on VLV  (at least for HDMI/EDP panels).

> What happens if we set up the pfit to use manual scaling ratios but
> configure both scaling ratios so that scaled image won't fill the
> active video region in either direction? Does it position the scaled
> image at coordinates 0,0 and simply scan black the rest of the time after
> it's run out of source pixel data? Or does it automagically center the
> image and scan black on both sides? Or does it fail in some way?
> 

Already tried that, but in vain. As per the VLV spec, the support for
Manual scaling ratio mode itself has been de-featured, so it didn't work
at all. So Auto/LetterBox/PillarBox modes are being supported.

As a next step tried to manipulate the Pipe timings, so as to produce
the borders on 4 sides of the panel. Similar to the PCH panel fitting
logic, kept the HSYNC/VSYNC pulse width same as well as the size of
HBLANK/VBLANK intervals, just manipulated the HYSNC/VSYNC and
HBLANK/VBLANK start & end points to create borders around the active
region. 

Tried to add Left/Right borders of 32 columns and Top/bottom borders of
30 lines. 
For that
HACTIVE=1920,  HBLANK START=1920, HSYNC START=1968, HYSNC END=2000,
HBLANK END=2080, HTOTAL=2080. 
was changed to 
HACTIVE=1856,    HBLANK START=1888, HSYNC START=1952, HYSNC END=1984,
HBLANK END=2048, HTOTAL=2080. 

And Similarly 
VACTIVE=1200, VBLANK START=1200, VSYNC START=1203, VYSNC END=1209,
VBLANK END=1235, VTOTAL=1235. 
was changed to
VACTIVE=1140, VBLANK START=1170, VSYNC START=1185, VYSNC END=1191,
VBLANK END=1205, VTOTAL=1235. 

After this manipulation, saw that the HMDI panel turned blank and showed
a "No Signal" message.
But for the same experiment, observed a different behavior with EDP
panel, that the display was active but the image was being shown in the
Top left part of the screen only, with the area outside active rectangle
having all junk data.

> > 
> > > > +
> > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > +		DRM_ERROR("Invalid border size input\n");
> > > > +		return;
> > > 
> > > This is clear sign here that we should do all this stuff in the compute
> > > config stage so that we can fail gracefully and tell userspace that things
> > > didn't work out.
> > > 
> > 
> > Actually this call to decide the panel fitter config, is being made in
> > the context of 'compute config' only. But it's originating from the
> > 'crtc set property' & not from the modeset.
> 
> We need to check things in both cases, and return an error if things
> can't work out.
> 

Can this be done in a next patch set i.e. adding return types to the functions
so that if there is an error in panel fitter configuration, it can be communicated
back to User space.

> > Do you mean to say that if done from the 'modeset' call, we can report
> > back the error to User space for an invalid border setting.
> > Actually currently the return type is 'void' only for the 2 existing
> > panel fitter config functions. 
> > Also we don't have a check in place for the max supported upscaling
> > ratio.
> 
> Is there an upscaling limit? I know there's a downscaling limit of IIRC
> 1.125x or something close to that. I don't think we check that either.
> 
Sorry my mistake, it's only the downscaling ratio which has a max value
of 1.125 i.e. PIPESRC cannot be downscaled by more than a factor of
1.125.

> > 
> > > > +	}
> > > > +
> > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > +
> > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > +		DRM_ERROR("width is too small\n");
> > > > +		return;
> > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > +		DRM_ERROR("height is too small\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> > > > +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> > > > +
> > > > +	pipe_config->pch_pfit.pos = (x << 16) | y;
> > > > +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> > > > +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> > > > +}
> > > > +
> > > >  /* adjusted_mode has been preset to be the panel's fixed mode */
> > > >  void
> > > >  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > >  
> > > >  	x = y = width = height = 0;
> > > >  
> > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > +	if (intel_crtc->pfit_enabled ||
> > > > +			intel_crtc->border_size)
> > > > +		return intel_pch_manual_panel_fitting(intel_crtc,
> > > > +						      pipe_config);
> > > > +
> > > >  	/* Native modes don't need fitting */
> > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
> > > >  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
> > > >  }
> > > >  
> > > > -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > > > -{
> > > > -	/*
> > > > -	 * Floating point operation is not supported. So the FACTOR
> > > > -	 * is defined, which can avoid the floating point computation
> > > > -	 * when calculating the panel ratio.
> > > > -	 */
> > > > -#define ACCURACY 12
> > > > -#define FACTOR (1 << ACCURACY)
> > > > -	u32 ratio = source * FACTOR / target;
> > > > -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > > > -}
> > > > -
> > > >  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> > > >  			      u32 *pfit_control)
> > > >  {
> > > > @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> > > >  	}
> > > >  }
> > > >  
> > > > +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > +				     struct intel_crtc_config *pipe_config)
> > > > +{
> > > > +	struct drm_device *dev = intel_crtc->base.dev;
> > > > +	u32 pfit_control = 0, border = 0;
> > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > +	struct drm_display_mode *adjusted_mode;
> > > > +	u32 tot_width, tot_height;
> > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > +	u32 dst_width, dst_height;
> > > > +
> > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > +
> > > > +	src_width = pipe_config->pipe_src_w;
> > > > +	src_height = pipe_config->pipe_src_h;
> > > > +
> > > > +	tot_width = adjusted_mode->hdisplay;
> > > > +	tot_height = adjusted_mode->vdisplay;
> > > > +
> > > > +	/*
> > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > +	 * output rectangle on screen
> > > > +	 */
> > > > +	dst_width = tot_width -
> > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > +	dst_height = tot_height -
> > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > > +
> > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > +		DRM_ERROR("Invalid border size input\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > +
> > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > +		DRM_ERROR("width is too small\n");
> > > > +		return;
> > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > +		DRM_ERROR("height is too small\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (dst_width != tot_width)
> > > > +		centre_horizontally(adjusted_mode, dst_width);
> > > > +	if (dst_height != tot_height)
> > > > +		centre_vertically(adjusted_mode, dst_height);
> > > > +
> > > > +	/* No scaling needed now, but still enable the panel fitter,
> > > > +	   as that will allow the User to subequently do the dynamic
> > > > +	   flipping of fbs of different resolutions */
> > > > +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> > > > +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> > > > +		BUG_ON(!intel_crtc->pfit_enabled);
> > > > +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> > > > +	}
> > > > +
> > > > +	border = LVDS_BORDER_ENABLE;
> > > > +
> > > > +	if (INTEL_INFO(dev)->gen >= 4) {
> > > > +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> > > > +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> > > > +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> > > > +	} else {
> > > > +		pfit_control |= (PFIT_ENABLE |
> > > > +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> > > > +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> > > > +	}
> > > > +
> > > > +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> > > > +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> > > > +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > > > +
> > > > +	pipe_config->gmch_pfit.control = pfit_control;
> > > > +	pipe_config->gmch_pfit.lvds_border_bits = border;
> > > > +}
> > > > +
> > > >  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > >  			      struct intel_crtc_config *pipe_config,
> > > >  			      int fitting_mode)
> > > > @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > >  
> > > >  	adjusted_mode = &pipe_config->adjusted_mode;
> > > >  
> > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > +	if (intel_crtc->pfit_enabled ||
> > > > +			intel_crtc->border_size)
> > > > +		return intel_gmch_manual_panel_fitting(intel_crtc,
> > > > +						       pipe_config);
> > > > +
> > > >  	/* Native modes don't need fitting */
> > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > -- 
> > > > 1.8.5.2
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > 
> 


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-11  3:04                 ` Akash Goel
@ 2014-04-11 11:42                   ` Ville Syrjälä
  2014-04-13 12:28                     ` Akash Goel
  2014-04-20 10:49                     ` [PATCH v5 3/4] " akash.goel
  0 siblings, 2 replies; 26+ messages in thread
From: Ville Syrjälä @ 2014-04-11 11:42 UTC (permalink / raw)
  To: Akash Goel; +Cc: intel-gfx

On Fri, Apr 11, 2014 at 08:34:08AM +0530, Akash Goel wrote:
> On Thu, 2014-04-10 at 14:34 +0300, Ville Syrjälä wrote:
> > On Thu, Apr 10, 2014 at 01:13:56PM +0530, Akash Goel wrote:
> > > On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
> > > > On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> > > > > From: Akash Goel <akash.goel@intel.com>
> > > > > 
> > > > > This patch adds a new drm crtc property for varying the size of
> > > > > the horizontal & vertical borers of the output/display window.
> > > > > This will control the output of Panel fitter.
> > > > > 
> > > > > v2: Added a new check for the invalid border size input
> > > > > 
> > > > > v3: Fixed bugs in output window calculation
> > > > > Removed superfluous checks
> > > > > 
> > > > > v4: Added the capability to forecfully enable the Panel fitter.
> > > > > The property value is of 64 bits, first 32 bits are used for
> > > > > border dimensions. The 33rd bit can be used to forcefully
> > > > > enable the panel fitter. This is useful for Panels which
> > > > > do not override the User specified Pipe timings.
> > > > > 
> > > > > Testcase: igt/kms_panel_fitter_test
> > > > > 
> > > > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
> > > > >  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
> > > > >  drivers/gpu/drm/i915/intel_drv.h     |   5 +
> > > > >  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
> > > > >  4 files changed, 211 insertions(+), 16 deletions(-)
> > > > > 
> > <snip>
> > > > > @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > > > >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> > > > >  }
> > > > >  
> > > > > +void
> > > > > +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > +			struct intel_crtc_config *pipe_config)
> > > > > +{
> > > > > +	struct drm_display_mode *adjusted_mode;
> > > > > +	int x, y;
> > > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > > +	u32 tot_width, tot_height;
> > > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > > +	u32 dst_width, dst_height;
> > > > > +
> > > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > > +
> > > > > +	src_width = pipe_config->pipe_src_w;
> > > > > +	src_height = pipe_config->pipe_src_h;
> > > > > +
> > > > > +	tot_width  = adjusted_mode->hdisplay;
> > > > > +	tot_height = adjusted_mode->vdisplay;
> > > > > +
> > > > > +	/*
> > > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > > +	 * output rectangle on screen
> > > > > +	 */
> > > > > +	dst_width = tot_width -
> > > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > > +	dst_height = tot_height -
> > > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > > 
> > > > I'm thinking that we should allow the user to specify each border width
> > > > individually rather than just assume left==right and top==bottom.
> > > > 
> > > 
> > > Sorry I thought that the Top/Bottom & left/Right borders would be
> > > symmetric only.
> > 
> > I don't see a reason to limit ourselves here.
> > 
> 
> Fine, will extend this property to set each border separately. 
> Can I use the 12 bits for each border value, as that shall be
> sufficient.

Maybe just add separate properties for each border value. We already
have similar properties for TV outputs.

> 
> > > Tried setting the borders on EDP & HDMI panels by manipulating the Pipe
> > > timings (through the logic used in 'centre_horizontally' &
> > > 'centre_vertically' functions), but it didn't work.
> > > Is this logic effective for the LVDS panel only ?
> > 
> > Could be. Certainly the border enable bit is there only for LVDS. The
> > gmch panel fitter isn't very flexible so it's possible we can't
> > actually make it do many of the things the pch pfit can do.
> > 
> 
> Yes the GMCH panel fitter function is not equally capable as the PCH
> counterpart. Here except the LVDS panel, it seems that borders cannot be
> realized on any other panel, just via the manipulation of Pipe timings
> (the way it can be done in PCH one).
> For the same reason the 'Center' Panel fitting mode of "scaling mode"
> property is not working on VLV  (at least for HDMI/EDP panels).
> 
> > What happens if we set up the pfit to use manual scaling ratios but
> > configure both scaling ratios so that scaled image won't fill the
> > active video region in either direction? Does it position the scaled
> > image at coordinates 0,0 and simply scan black the rest of the time after
> > it's run out of source pixel data? Or does it automagically center the
> > image and scan black on both sides? Or does it fail in some way?
> > 
> 
> Already tried that, but in vain. As per the VLV spec, the support for
> Manual scaling ratio mode itself has been de-featured, so it didn't work
> at all. So Auto/LetterBox/PillarBox modes are being supported.

I see.

> 
> As a next step tried to manipulate the Pipe timings, so as to produce
> the borders on 4 sides of the panel. Similar to the PCH panel fitting
> logic, kept the HSYNC/VSYNC pulse width same as well as the size of
> HBLANK/VBLANK intervals, just manipulated the HYSNC/VSYNC and
> HBLANK/VBLANK start & end points to create borders around the active
> region. 
> 
> Tried to add Left/Right borders of 32 columns and Top/bottom borders of
> 30 lines. 
> For that
> HACTIVE=1920,  HBLANK START=1920, HSYNC START=1968, HYSNC END=2000,
> HBLANK END=2080, HTOTAL=2080. 
> was changed to 
> HACTIVE=1856,    HBLANK START=1888, HSYNC START=1952, HYSNC END=1984,
> HBLANK END=2048, HTOTAL=2080. 
> 
> And Similarly 
> VACTIVE=1200, VBLANK START=1200, VSYNC START=1203, VYSNC END=1209,
> VBLANK END=1235, VTOTAL=1235. 
> was changed to
> VACTIVE=1140, VBLANK START=1170, VSYNC START=1185, VYSNC END=1191,
> VBLANK END=1205, VTOTAL=1235. 

Your sync values seem to be a bit off. AFAICS they should be:
 hsync_start = 1936, hsync_end = 1968
 vsync_start = 1173, vsync_end = 1179

> 
> After this manipulation, saw that the HMDI panel turned blank and showed
> a "No Signal" message.

Might be good to repeat with fixed sync positions.

There's also some kind of border enable bit in the sdvo/hdmi control
register. No idea if that does anything useful.

> But for the same experiment, observed a different behavior with EDP
> panel, that the display was active but the image was being shown in the
> Top left part of the screen only, with the area outside active rectangle
> having all junk data.

:(

I suppose it's still likely that things won't actually work even w/ the
sync pulses in the correct position. And that would mean we can't support
borders on non-LVDS outputs on gmch platforms.

And that would also mean we already have a bug with the current
scaling_mode property on VLV eDP.

> 
> > > 
> > > > > +
> > > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > > +		DRM_ERROR("Invalid border size input\n");
> > > > > +		return;
> > > > 
> > > > This is clear sign here that we should do all this stuff in the compute
> > > > config stage so that we can fail gracefully and tell userspace that things
> > > > didn't work out.
> > > > 
> > > 
> > > Actually this call to decide the panel fitter config, is being made in
> > > the context of 'compute config' only. But it's originating from the
> > > 'crtc set property' & not from the modeset.
> > 
> > We need to check things in both cases, and return an error if things
> > can't work out.
> > 
> 
> Can this be done in a next patch set i.e. adding return types to the functions
> so that if there is an error in panel fitter configuration, it can be communicated
> back to User space.

I don't see a reason for delaying it.

> 
> > > Do you mean to say that if done from the 'modeset' call, we can report
> > > back the error to User space for an invalid border setting.
> > > Actually currently the return type is 'void' only for the 2 existing
> > > panel fitter config functions. 
> > > Also we don't have a check in place for the max supported upscaling
> > > ratio.
> > 
> > Is there an upscaling limit? I know there's a downscaling limit of IIRC
> > 1.125x or something close to that. I don't think we check that either.
> > 
> Sorry my mistake, it's only the downscaling ratio which has a max value
> of 1.125 i.e. PIPESRC cannot be downscaled by more than a factor of
> 1.125.
> 
> > > 
> > > > > +	}
> > > > > +
> > > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > > +
> > > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > +		DRM_ERROR("width is too small\n");
> > > > > +		return;
> > > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > +		DRM_ERROR("height is too small\n");
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > > +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> > > > > +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> > > > > +
> > > > > +	pipe_config->pch_pfit.pos = (x << 16) | y;
> > > > > +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> > > > > +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> > > > > +}
> > > > > +
> > > > >  /* adjusted_mode has been preset to be the panel's fixed mode */
> > > > >  void
> > > > >  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > >  
> > > > >  	x = y = width = height = 0;
> > > > >  
> > > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > > +	if (intel_crtc->pfit_enabled ||
> > > > > +			intel_crtc->border_size)
> > > > > +		return intel_pch_manual_panel_fitting(intel_crtc,
> > > > > +						      pipe_config);
> > > > > +
> > > > >  	/* Native modes don't need fitting */
> > > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > > @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
> > > > >  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
> > > > >  }
> > > > >  
> > > > > -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > > > > -{
> > > > > -	/*
> > > > > -	 * Floating point operation is not supported. So the FACTOR
> > > > > -	 * is defined, which can avoid the floating point computation
> > > > > -	 * when calculating the panel ratio.
> > > > > -	 */
> > > > > -#define ACCURACY 12
> > > > > -#define FACTOR (1 << ACCURACY)
> > > > > -	u32 ratio = source * FACTOR / target;
> > > > > -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > > > > -}
> > > > > -
> > > > >  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> > > > >  			      u32 *pfit_control)
> > > > >  {
> > > > > @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> > > > >  	}
> > > > >  }
> > > > >  
> > > > > +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > +				     struct intel_crtc_config *pipe_config)
> > > > > +{
> > > > > +	struct drm_device *dev = intel_crtc->base.dev;
> > > > > +	u32 pfit_control = 0, border = 0;
> > > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > > +	struct drm_display_mode *adjusted_mode;
> > > > > +	u32 tot_width, tot_height;
> > > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > > +	u32 dst_width, dst_height;
> > > > > +
> > > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > > +
> > > > > +	src_width = pipe_config->pipe_src_w;
> > > > > +	src_height = pipe_config->pipe_src_h;
> > > > > +
> > > > > +	tot_width = adjusted_mode->hdisplay;
> > > > > +	tot_height = adjusted_mode->vdisplay;
> > > > > +
> > > > > +	/*
> > > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > > +	 * output rectangle on screen
> > > > > +	 */
> > > > > +	dst_width = tot_width -
> > > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > > +	dst_height = tot_height -
> > > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > > > +
> > > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > > +		DRM_ERROR("Invalid border size input\n");
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > > +
> > > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > +		DRM_ERROR("width is too small\n");
> > > > > +		return;
> > > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > +		DRM_ERROR("height is too small\n");
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > > +	if (dst_width != tot_width)
> > > > > +		centre_horizontally(adjusted_mode, dst_width);
> > > > > +	if (dst_height != tot_height)
> > > > > +		centre_vertically(adjusted_mode, dst_height);
> > > > > +
> > > > > +	/* No scaling needed now, but still enable the panel fitter,
> > > > > +	   as that will allow the User to subequently do the dynamic
> > > > > +	   flipping of fbs of different resolutions */
> > > > > +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> > > > > +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> > > > > +		BUG_ON(!intel_crtc->pfit_enabled);
> > > > > +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> > > > > +	}
> > > > > +
> > > > > +	border = LVDS_BORDER_ENABLE;
> > > > > +
> > > > > +	if (INTEL_INFO(dev)->gen >= 4) {
> > > > > +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> > > > > +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> > > > > +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> > > > > +	} else {
> > > > > +		pfit_control |= (PFIT_ENABLE |
> > > > > +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> > > > > +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> > > > > +	}
> > > > > +
> > > > > +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> > > > > +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> > > > > +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > > > > +
> > > > > +	pipe_config->gmch_pfit.control = pfit_control;
> > > > > +	pipe_config->gmch_pfit.lvds_border_bits = border;
> > > > > +}
> > > > > +
> > > > >  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > >  			      struct intel_crtc_config *pipe_config,
> > > > >  			      int fitting_mode)
> > > > > @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > >  
> > > > >  	adjusted_mode = &pipe_config->adjusted_mode;
> > > > >  
> > > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > > +	if (intel_crtc->pfit_enabled ||
> > > > > +			intel_crtc->border_size)
> > > > > +		return intel_gmch_manual_panel_fitting(intel_crtc,
> > > > > +						       pipe_config);
> > > > > +
> > > > >  	/* Native modes don't need fitting */
> > > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > > -- 
> > > > > 1.8.5.2
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > > 
> > 
> 

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-11 11:42                   ` Ville Syrjälä
@ 2014-04-13 12:28                     ` Akash Goel
  2014-04-13 17:12                       ` Daniel Vetter
  2014-04-20 10:49                     ` [PATCH v5 3/4] " akash.goel
  1 sibling, 1 reply; 26+ messages in thread
From: Akash Goel @ 2014-04-13 12:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Fri, 2014-04-11 at 14:42 +0300, Ville Syrjälä wrote:
> On Fri, Apr 11, 2014 at 08:34:08AM +0530, Akash Goel wrote:
> > On Thu, 2014-04-10 at 14:34 +0300, Ville Syrjälä wrote:
> > > On Thu, Apr 10, 2014 at 01:13:56PM +0530, Akash Goel wrote:
> > > > On Tue, 2014-04-08 at 19:28 +0300, Ville Syrjälä wrote:
> > > > > On Wed, Mar 26, 2014 at 09:25:12AM +0530, akash.goel@intel.com wrote:
> > > > > > From: Akash Goel <akash.goel@intel.com>
> > > > > > 
> > > > > > This patch adds a new drm crtc property for varying the size of
> > > > > > the horizontal & vertical borers of the output/display window.
> > > > > > This will control the output of Panel fitter.
> > > > > > 
> > > > > > v2: Added a new check for the invalid border size input
> > > > > > 
> > > > > > v3: Fixed bugs in output window calculation
> > > > > > Removed superfluous checks
> > > > > > 
> > > > > > v4: Added the capability to forecfully enable the Panel fitter.
> > > > > > The property value is of 64 bits, first 32 bits are used for
> > > > > > border dimensions. The 33rd bit can be used to forcefully
> > > > > > enable the panel fitter. This is useful for Panels which
> > > > > > do not override the User specified Pipe timings.
> > > > > > 
> > > > > > Testcase: igt/kms_panel_fitter_test
> > > > > > 
> > > > > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_drv.h      |   7 ++
> > > > > >  drivers/gpu/drm/i915/intel_display.c |  39 +++++++-
> > > > > >  drivers/gpu/drm/i915/intel_drv.h     |   5 +
> > > > > >  drivers/gpu/drm/i915/intel_panel.c   | 176 ++++++++++++++++++++++++++++++++---
> > > > > >  4 files changed, 211 insertions(+), 16 deletions(-)
> > > > > > 
> > > <snip>
> > > > > > @@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > > > > >  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> > > > > >  }
> > > > > >  
> > > > > > +void
> > > > > > +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > > +			struct intel_crtc_config *pipe_config)
> > > > > > +{
> > > > > > +	struct drm_display_mode *adjusted_mode;
> > > > > > +	int x, y;
> > > > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > > > +	u32 tot_width, tot_height;
> > > > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > > > +	u32 dst_width, dst_height;
> > > > > > +
> > > > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > > > +
> > > > > > +	src_width = pipe_config->pipe_src_w;
> > > > > > +	src_height = pipe_config->pipe_src_h;
> > > > > > +
> > > > > > +	tot_width  = adjusted_mode->hdisplay;
> > > > > > +	tot_height = adjusted_mode->vdisplay;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > > > +	 * output rectangle on screen
> > > > > > +	 */
> > > > > > +	dst_width = tot_width -
> > > > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > > > +	dst_height = tot_height -
> > > > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > > > 
> > > > > I'm thinking that we should allow the user to specify each border width
> > > > > individually rather than just assume left==right and top==bottom.
> > > > > 
> > > > 
> > > > Sorry I thought that the Top/Bottom & left/Right borders would be
> > > > symmetric only.
> > > 
> > > I don't see a reason to limit ourselves here.
> > > 
> > 
> > Fine, will extend this property to set each border separately. 
> > Can I use the 12 bits for each border value, as that shall be
> > sufficient.
> 
> Maybe just add separate properties for each border value. We already
> have similar properties for TV outputs.
> 
ok so should define 4 new properties like "left margin", "right margin",
"top margin", "bottom margin" already defined for TV.

> > 
> > > > Tried setting the borders on EDP & HDMI panels by manipulating the Pipe
> > > > timings (through the logic used in 'centre_horizontally' &
> > > > 'centre_vertically' functions), but it didn't work.
> > > > Is this logic effective for the LVDS panel only ?
> > > 
> > > Could be. Certainly the border enable bit is there only for LVDS. The
> > > gmch panel fitter isn't very flexible so it's possible we can't
> > > actually make it do many of the things the pch pfit can do.
> > > 
> > 
> > Yes the GMCH panel fitter function is not equally capable as the PCH
> > counterpart. Here except the LVDS panel, it seems that borders cannot be
> > realized on any other panel, just via the manipulation of Pipe timings
> > (the way it can be done in PCH one).
> > For the same reason the 'Center' Panel fitting mode of "scaling mode"
> > property is not working on VLV  (at least for HDMI/EDP panels).
> > 
> > > What happens if we set up the pfit to use manual scaling ratios but
> > > configure both scaling ratios so that scaled image won't fill the
> > > active video region in either direction? Does it position the scaled
> > > image at coordinates 0,0 and simply scan black the rest of the time after
> > > it's run out of source pixel data? Or does it automagically center the
> > > image and scan black on both sides? Or does it fail in some way?
> > > 
> > 
> > Already tried that, but in vain. As per the VLV spec, the support for
> > Manual scaling ratio mode itself has been de-featured, so it didn't work
> > at all. So Auto/LetterBox/PillarBox modes are being supported.
> 
> I see.
> 
> > 
> > As a next step tried to manipulate the Pipe timings, so as to produce
> > the borders on 4 sides of the panel. Similar to the PCH panel fitting
> > logic, kept the HSYNC/VSYNC pulse width same as well as the size of
> > HBLANK/VBLANK intervals, just manipulated the HYSNC/VSYNC and
> > HBLANK/VBLANK start & end points to create borders around the active
> > region. 
> > 
> > Tried to add Left/Right borders of 32 columns and Top/bottom borders of
> > 30 lines. 
> > For that
> > HACTIVE=1920,  HBLANK START=1920, HSYNC START=1968, HYSNC END=2000,
> > HBLANK END=2080, HTOTAL=2080. 
> > was changed to 
> > HACTIVE=1856,    HBLANK START=1888, HSYNC START=1952, HYSNC END=1984,
> > HBLANK END=2048, HTOTAL=2080. 
> > 
> > And Similarly 
> > VACTIVE=1200, VBLANK START=1200, VSYNC START=1203, VYSNC END=1209,
> > VBLANK END=1235, VTOTAL=1235. 
> > was changed to
> > VACTIVE=1140, VBLANK START=1170, VSYNC START=1185, VYSNC END=1191,
> > VBLANK END=1205, VTOTAL=1235. 
> 
> Your sync values seem to be a bit off. AFAICS they should be:
>  hsync_start = 1936, hsync_end = 1968
>  vsync_start = 1173, vsync_end = 1179
> 

Actually these are the values outputted by 'centre_horizontally' &
'centre_vertically' functions, where the logic is to position the
hsyc/vsync pulses in the middle of the corresponding hblank/vblank
intervals.

Thus the HSYNC START=1952, HYSNC END=1984 (pulse of 32 pixels) is lying
in the middle of HBLANK START=1888, HBLANK END=2048 interval (160
pixels).

> > 
> > After this manipulation, saw that the HMDI panel turned blank and showed
> > a "No Signal" message.
> 
> Might be good to repeat with fixed sync positions.
> 
> There's also some kind of border enable bit in the sdvo/hdmi control
> register. No idea if that does anything useful.
> 
> > But for the same experiment, observed a different behavior with EDP
> > panel, that the display was active but the image was being shown in the
> > Top left part of the screen only, with the area outside active rectangle
> > having all junk data.
> 
> :(
> 
> I suppose it's still likely that things won't actually work even w/ the
> sync pulses in the correct position. And that would mean we can't support
> borders on non-LVDS outputs on gmch platforms.
> 
> And that would also mean we already have a bug with the current
> scaling_mode property on VLV eDP.
> 

Yes, the 'Center' Panel fitting mode of "scaling mode" property is not
working on VLV (for HDMI/EDP panels).

> > 
> > > > 
> > > > > > +
> > > > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > > > +		DRM_ERROR("Invalid border size input\n");
> > > > > > +		return;
> > > > > 
> > > > > This is clear sign here that we should do all this stuff in the compute
> > > > > config stage so that we can fail gracefully and tell userspace that things
> > > > > didn't work out.
> > > > > 
> > > > 
> > > > Actually this call to decide the panel fitter config, is being made in
> > > > the context of 'compute config' only. But it's originating from the
> > > > 'crtc set property' & not from the modeset.
> > > 
> > > We need to check things in both cases, and return an error if things
> > > can't work out.
> > > 
> > 
> > Can this be done in a next patch set i.e. adding return types to the functions
> > so that if there is an error in panel fitter configuration, it can be communicated
> > back to User space.
> 
> I don't see a reason for delaying it.
> 
> > 
> > > > Do you mean to say that if done from the 'modeset' call, we can report
> > > > back the error to User space for an invalid border setting.
> > > > Actually currently the return type is 'void' only for the 2 existing
> > > > panel fitter config functions. 
> > > > Also we don't have a check in place for the max supported upscaling
> > > > ratio.
> > > 
> > > Is there an upscaling limit? I know there's a downscaling limit of IIRC
> > > 1.125x or something close to that. I don't think we check that either.
> > > 
> > Sorry my mistake, it's only the downscaling ratio which has a max value
> > of 1.125 i.e. PIPESRC cannot be downscaled by more than a factor of
> > 1.125.
> > 
> > > > 
> > > > > > +	}
> > > > > > +
> > > > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > > > +
> > > > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > > +		DRM_ERROR("width is too small\n");
> > > > > > +		return;
> > > > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > > +		DRM_ERROR("height is too small\n");
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > > +	x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> > > > > > +	y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> > > > > > +
> > > > > > +	pipe_config->pch_pfit.pos = (x << 16) | y;
> > > > > > +	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> > > > > > +	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
> > > > > > +}
> > > > > > +
> > > > > >  /* adjusted_mode has been preset to be the panel's fixed mode */
> > > > > >  void
> > > > > >  intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > > @@ -55,6 +124,13 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > >  
> > > > > >  	x = y = width = height = 0;
> > > > > >  
> > > > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > > > +	if (intel_crtc->pfit_enabled ||
> > > > > > +			intel_crtc->border_size)
> > > > > > +		return intel_pch_manual_panel_fitting(intel_crtc,
> > > > > > +						      pipe_config);
> > > > > > +
> > > > > >  	/* Native modes don't need fitting */
> > > > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > > > @@ -157,19 +233,6 @@ centre_vertically(struct drm_display_mode *mode,
> > > > > >  	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
> > > > > >  }
> > > > > >  
> > > > > > -static inline u32 panel_fitter_scaling(u32 source, u32 target)
> > > > > > -{
> > > > > > -	/*
> > > > > > -	 * Floating point operation is not supported. So the FACTOR
> > > > > > -	 * is defined, which can avoid the floating point computation
> > > > > > -	 * when calculating the panel ratio.
> > > > > > -	 */
> > > > > > -#define ACCURACY 12
> > > > > > -#define FACTOR (1 << ACCURACY)
> > > > > > -	u32 ratio = source * FACTOR / target;
> > > > > > -	return (FACTOR * ratio + FACTOR/2) / FACTOR;
> > > > > > -}
> > > > > > -
> > > > > >  static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
> > > > > >  			      u32 *pfit_control)
> > > > > >  {
> > > > > > @@ -247,6 +310,86 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
> > > > > >  	}
> > > > > >  }
> > > > > >  
> > > > > > +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > > +				     struct intel_crtc_config *pipe_config)
> > > > > > +{
> > > > > > +	struct drm_device *dev = intel_crtc->base.dev;
> > > > > > +	u32 pfit_control = 0, border = 0;
> > > > > > +	u32 pf_horizontal_ratio, pf_vertical_ratio;
> > > > > > +	struct drm_display_mode *adjusted_mode;
> > > > > > +	u32 tot_width, tot_height;
> > > > > > +	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> > > > > > +	u32 dst_width, dst_height;
> > > > > > +
> > > > > > +	adjusted_mode = &pipe_config->adjusted_mode;
> > > > > > +
> > > > > > +	src_width = pipe_config->pipe_src_w;
> > > > > > +	src_height = pipe_config->pipe_src_h;
> > > > > > +
> > > > > > +	tot_width = adjusted_mode->hdisplay;
> > > > > > +	tot_height = adjusted_mode->vdisplay;
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
> > > > > > +	 * region. So (HACTIVE - Left border - Right Border) *
> > > > > > +	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
> > > > > > +	 * output rectangle on screen
> > > > > > +	 */
> > > > > > +	dst_width = tot_width -
> > > > > > +			(((intel_crtc->border_size >> 16) & 0xffff) * 2);
> > > > > > +	dst_height = tot_height -
> > > > > > +			((intel_crtc->border_size & 0xffff) * 2);
> > > > > > +
> > > > > > +	if ((dst_width == 0) || (dst_height == 0)) {
> > > > > > +		DRM_ERROR("Invalid border size input\n");
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > > +	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> > > > > > +	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> > > > > > +
> > > > > > +	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > > +		DRM_ERROR("width is too small\n");
> > > > > > +		return;
> > > > > > +	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> > > > > > +		DRM_ERROR("height is too small\n");
> > > > > > +		return;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (dst_width != tot_width)
> > > > > > +		centre_horizontally(adjusted_mode, dst_width);
> > > > > > +	if (dst_height != tot_height)
> > > > > > +		centre_vertically(adjusted_mode, dst_height);
> > > > > > +
> > > > > > +	/* No scaling needed now, but still enable the panel fitter,
> > > > > > +	   as that will allow the User to subequently do the dynamic
> > > > > > +	   flipping of fbs of different resolutions */
> > > > > > +	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> > > > > > +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
> > > > > > +		BUG_ON(!intel_crtc->pfit_enabled);
> > > > > > +		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
> > > > > > +	}
> > > > > > +
> > > > > > +	border = LVDS_BORDER_ENABLE;
> > > > > > +
> > > > > > +	if (INTEL_INFO(dev)->gen >= 4) {
> > > > > > +		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
> > > > > > +		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
> > > > > > +		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
> > > > > > +	} else {
> > > > > > +		pfit_control |= (PFIT_ENABLE |
> > > > > > +				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
> > > > > > +				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
> > > > > > +	}
> > > > > > +
> > > > > > +	/* Make sure pre-965 set dither correctly for 18bpp panels. */
> > > > > > +	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
> > > > > > +		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > > > > > +
> > > > > > +	pipe_config->gmch_pfit.control = pfit_control;
> > > > > > +	pipe_config->gmch_pfit.lvds_border_bits = border;
> > > > > > +}
> > > > > > +
> > > > > >  void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > >  			      struct intel_crtc_config *pipe_config,
> > > > > >  			      int fitting_mode)
> > > > > > @@ -257,6 +400,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> > > > > >  
> > > > > >  	adjusted_mode = &pipe_config->adjusted_mode;
> > > > > >  
> > > > > > +	/* check if User wants to apply the borders, or wants to forcefully
> > > > > > +	   enable the panel fitter, otherwise fall through the regular path */
> > > > > > +	if (intel_crtc->pfit_enabled ||
> > > > > > +			intel_crtc->border_size)
> > > > > > +		return intel_gmch_manual_panel_fitting(intel_crtc,
> > > > > > +						       pipe_config);
> > > > > > +
> > > > > >  	/* Native modes don't need fitting */
> > > > > >  	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> > > > > >  	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
> > > > > > -- 
> > > > > > 1.8.5.2
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > > 
> > > > 
> > > 
> > 
> 


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 3/3] drm/i915: New drm crtc property for varying the size of borders
  2014-04-13 12:28                     ` Akash Goel
@ 2014-04-13 17:12                       ` Daniel Vetter
  0 siblings, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2014-04-13 17:12 UTC (permalink / raw)
  To: Akash Goel; +Cc: intel-gfx

On Sun, Apr 13, 2014 at 2:28 PM, Akash Goel <akash.goel@intel.com> wrote:
> Yes, the 'Center' Panel fitting mode of "scaling mode" property is not
> working on VLV (for HDMI/EDP panels).

That sounds like a really good reason for a stab at a CRC based
testcase for all this - we've blown up the different upscaling modes
way too often already.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v5 3/4] drm/i915: New drm crtc property for varying the size of borders
  2014-04-11 11:42                   ` Ville Syrjälä
  2014-04-13 12:28                     ` Akash Goel
@ 2014-04-20 10:49                     ` akash.goel
  1 sibling, 0 replies; 26+ messages in thread
From: akash.goel @ 2014-04-20 10:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

From: Akash Goel <akash.goel@intel.com>

This patch adds a new drm crtc property for varying the size of
the horizontal & vertical borers of the output/display window.
This will control the output of Panel fitter.

v2: Added a new check for the invalid border size input

v3: Fixed bugs in output window calculation
Removed superfluous checks

v4: Added the capability to forecfully enable the Panel fitter.
The property value is of 64 bits, first 32 bits are used for
border dimensions. The 33rd bit can be used to forcefully
enable the panel fitter. This is useful for Panels which
do not override the User specified Pipe timings.

v5: Splitted the single border property into 4 separate
properties so as to allow a control on the size of each border
left/top/bottom/right (Ville)
Reverted the changes done in v4 of the patch, which provided
forceful enabling of panel fitter.
Also refactored based on latest codebase.

Testcase: igt/kms_panel_fitter_test

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  10 ++
 drivers/gpu/drm/i915/intel_display.c | 107 ++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  12 ++
 drivers/gpu/drm/i915/intel_panel.c   | 217 ++++++++++++++++++++++++++++++++---
 4 files changed, 331 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104a232..dfe3e03 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1447,6 +1447,16 @@ struct drm_i915_private {
 	 */
 	struct drm_property *input_size_property;
 
+	/*
+	 * Properties to dynamically vary the size of the
+	 * borders. This will indirectly control the size
+	 * of the display window i.e Panel fitter output
+	 */
+	struct drm_property *left_border_property;
+	struct drm_property *right_border_property;
+	struct drm_property *top_border_property;
+	struct drm_property *bottom_border_property;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5484ae2..8ff99bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10438,15 +10438,57 @@ static void intel_create_crtc_properties(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	/* Create properties*/
 	if (!dev_priv->input_size_property)
 		dev_priv->input_size_property =
 			drm_property_create_range(dev, 0, "input size",
 						  0, UINT_MAX);
 
+	if (!dev_priv->left_border_property)
+		dev_priv->left_border_property =
+			drm_property_create_range(dev, 0, "left border",
+						0, (uint64_t)MAX_BORDER_VALUE);
+
+	if (!dev_priv->right_border_property)
+		dev_priv->right_border_property =
+			drm_property_create_range(dev, 0, "right border",
+						0, (uint64_t)MAX_BORDER_VALUE);
+
+	if (!dev_priv->top_border_property)
+		dev_priv->top_border_property =
+			drm_property_create_range(dev, 0, "top border",
+						0, (uint64_t)MAX_BORDER_VALUE);
+
+	if (!dev_priv->bottom_border_property)
+		dev_priv->bottom_border_property =
+			drm_property_create_range(dev, 0, "bottom border",
+						0, (uint64_t)MAX_BORDER_VALUE);
+
+	/* Attach to properties*/
 	if (dev_priv->input_size_property)
 		drm_object_attach_property(&intel_crtc->base.base,
 					   dev_priv->input_size_property,
 					   0);
+
+	if (dev_priv->left_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->left_border_property,
+					   0);
+
+	if (dev_priv->right_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->right_border_property,
+					   0);
+
+	if (dev_priv->top_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->top_border_property,
+					   0);
+
+	if (dev_priv->bottom_border_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					   dev_priv->bottom_border_property,
+					   0);
 }
 
 static int intel_crtc_set_property(struct drm_crtc *crtc,
@@ -10455,7 +10497,6 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int ret = -ENOENT;
 
 	if (property == dev_priv->input_size_property) {
 		int new_width = (int)((val >> 16) & 0xffff);
@@ -10497,7 +10538,69 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
-	return ret;
+	if (property == dev_priv->left_border_property) {
+		if (val == (uint64_t)intel_crtc->border[PANEL_BORDER_LEFT])
+			return 0;
+		if (val & 1) {
+			DRM_ERROR("Odd border value not supported\n");
+			return -EINVAL;
+		}
+
+		intel_crtc->border[PANEL_BORDER_LEFT] =	(uint32_t)val;
+		goto done;
+	}
+
+	if (property == dev_priv->right_border_property) {
+		if (val == (uint64_t)intel_crtc->border[PANEL_BORDER_RIGHT])
+			return 0;
+		if (val & 1) {
+			DRM_ERROR("Odd border value not supported\n");
+			return -EINVAL;
+		}
+
+		intel_crtc->border[PANEL_BORDER_RIGHT] = (uint32_t)val;
+		goto done;
+	}
+
+	if (property == dev_priv->top_border_property) {
+		if (val == (uint64_t)intel_crtc->border[PANEL_BORDER_TOP])
+			return 0;
+		if (val & 1) {
+			DRM_ERROR("Odd border value not supported\n");
+			return -EINVAL;
+		}
+
+		intel_crtc->border[PANEL_BORDER_TOP] = (uint32_t)val;
+		goto done;
+	}
+
+	if (property == dev_priv->bottom_border_property) {
+		if (val == (uint64_t)intel_crtc->border[PANEL_BORDER_BOTTOM])
+			return 0;
+		if (val & 1) {
+			DRM_ERROR("Odd border value not supported\n");
+			return -EINVAL;
+		}
+
+		intel_crtc->border[PANEL_BORDER_BOTTOM] = (uint32_t)val;
+		goto done;
+	}
+
+	return -EINVAL;
+
+done:
+	if (crtc) {
+		intel_crtc->apply_borders =
+			((intel_crtc->border[PANEL_BORDER_LEFT] != 0) ||
+			 (intel_crtc->border[PANEL_BORDER_RIGHT] != 0) ||
+			 (intel_crtc->border[PANEL_BORDER_TOP] != 0) ||
+			 (intel_crtc->border[PANEL_BORDER_BOTTOM] != 0));
+
+		intel_crtc_restore_mode(crtc);
+		return 0;
+	}
+
+	return -EINVAL;
 }
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c551472..f00d075 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -109,6 +109,14 @@
 #define INTEL_DSI_COMMAND_MODE	0
 #define INTEL_DSI_VIDEO_MODE	1
 
+#define MAX_BORDER_VALUE 256
+enum panel_border {
+	PANEL_BORDER_LEFT = 0,
+	PANEL_BORDER_RIGHT,
+	PANEL_BORDER_TOP,
+	PANEL_BORDER_BOTTOM,
+};
+
 struct intel_framebuffer {
 	struct drm_framebuffer base;
 	struct drm_i915_gem_object *obj;
@@ -395,6 +403,10 @@ struct intel_crtc {
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
 
+	/* border info for the output/display window */
+	uint32_t border[4];
+	bool apply_borders;
+
 	/* per-pipe watermark state */
 	struct {
 		/* watermarks currently being used  */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 44ad415..3dbb774 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -29,10 +29,25 @@
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format */
+#define MAX_DOWNSCALE_RATIO  (0x9 << 9)
 
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
+static inline u32 panel_fitter_scaling(u32 source, u32 target)
+{
+	/*
+	 * Floating point operation is not supported. So the FACTOR
+	 * is defined, which can avoid the floating point computation
+	 * when calculating the panel ratio.
+	 */
+#define ACCURACY 12
+#define FACTOR (1 << ACCURACY)
+	u32 ratio = source * FACTOR / target;
+	return (FACTOR * ratio + FACTOR/2) / FACTOR;
+}
+
 void
 intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -42,6 +57,60 @@ intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
+void
+intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+			struct intel_crtc_config *pipe_config)
+{
+	struct drm_display_mode *adjusted_mode;
+	int x, y;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width  = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width - intel_crtc->border[PANEL_BORDER_LEFT] -
+				intel_crtc->border[PANEL_BORDER_RIGHT];
+	dst_height = tot_height - intel_crtc->border[PANEL_BORDER_TOP] -
+				intel_crtc->border[PANEL_BORDER_BOTTOM];
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	x = intel_crtc->border[PANEL_BORDER_LEFT];
+	y = intel_crtc->border[PANEL_BORDER_TOP];
+
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+}
+
 /* adjusted_mode has been preset to be the panel's fixed mode */
 void
 intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
@@ -55,6 +124,12 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	x = y = width = height = 0;
 
+	/* check if User wants to apply the borders, otherwise fall
+	   through the regular path */
+	if (intel_crtc->apply_borders)
+		return intel_pch_manual_panel_fitting(intel_crtc,
+						      pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
@@ -115,6 +190,50 @@ done:
 }
 
 static void
+apply_horizontal_borders(struct drm_display_mode *mode,
+		    int *border)
+{
+	u32 width, sync_pos, blank_width, sync_width;
+
+	/* keep the hsync and hblank widths constant */
+	sync_width = mode->crtc_hsync_end - mode->crtc_hsync_start;
+	blank_width = mode->crtc_hblank_end - mode->crtc_hblank_start;
+	sync_pos = (blank_width - sync_width + 1) / 2;
+
+	width = mode->hdisplay - border[PANEL_BORDER_LEFT] -
+				 border[PANEL_BORDER_RIGHT];
+
+	mode->crtc_hdisplay = width;
+	mode->crtc_hblank_start = width + border[PANEL_BORDER_RIGHT];
+	mode->crtc_hblank_end = mode->crtc_hblank_start + blank_width;
+
+	mode->crtc_hsync_start = mode->crtc_hblank_start + sync_pos;
+	mode->crtc_hsync_end = mode->crtc_hsync_start + sync_width;
+}
+
+static void
+apply_vertical_borders(struct drm_display_mode *mode,
+		    int *border)
+{
+	u32 height, sync_pos, blank_width, sync_width;
+
+	/* keep the vsync and vblank widths constant */
+	sync_width = mode->crtc_vsync_end - mode->crtc_vsync_start;
+	blank_width = mode->crtc_vblank_end - mode->crtc_vblank_start;
+	sync_pos = (blank_width - sync_width + 1) / 2;
+
+	height = mode->vdisplay - border[PANEL_BORDER_TOP] -
+				  border[PANEL_BORDER_BOTTOM];
+
+	mode->crtc_vdisplay = height;
+	mode->crtc_vblank_start = height + border[PANEL_BORDER_BOTTOM];
+	mode->crtc_vblank_end = mode->crtc_vblank_start + blank_width;
+
+	mode->crtc_vsync_start = mode->crtc_vblank_start + sync_pos;
+	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
+}
+
+static void
 centre_horizontally(struct drm_display_mode *mode,
 		    int width)
 {
@@ -157,19 +276,6 @@ centre_vertically(struct drm_display_mode *mode,
 	mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width;
 }
 
-static inline u32 panel_fitter_scaling(u32 source, u32 target)
-{
-	/*
-	 * Floating point operation is not supported. So the FACTOR
-	 * is defined, which can avoid the floating point computation
-	 * when calculating the panel ratio.
-	 */
-#define ACCURACY 12
-#define FACTOR (1 << ACCURACY)
-	u32 ratio = source * FACTOR / target;
-	return (FACTOR * ratio + FACTOR/2) / FACTOR;
-}
-
 static void i965_scale_aspect(struct intel_crtc_config *pipe_config,
 			      u32 *pfit_control)
 {
@@ -247,6 +353,85 @@ static void i9xx_scale_aspect(struct intel_crtc_config *pipe_config,
 	}
 }
 
+void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
+				     struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	u32 pfit_control = 0, border = 0;
+	u32 pf_horizontal_ratio, pf_vertical_ratio;
+	struct drm_display_mode *adjusted_mode;
+	u32 tot_width, tot_height;
+	u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
+	u32 dst_width, dst_height;
+
+	adjusted_mode = &pipe_config->adjusted_mode;
+
+	src_width = pipe_config->pipe_src_w;
+	src_height = pipe_config->pipe_src_h;
+
+	tot_width = adjusted_mode->hdisplay;
+	tot_height = adjusted_mode->vdisplay;
+
+	/*
+	 * Having non zero borders will reduce the size of 'HACTIVE/VACTIVE'
+	 * region. So (HACTIVE - Left border - Right Border) *
+	 * (VACTIVE  - Top Border  - Bottom border) will effectively be the
+	 * output rectangle on screen
+	 */
+	dst_width = tot_width - intel_crtc->border[PANEL_BORDER_LEFT] -
+				intel_crtc->border[PANEL_BORDER_RIGHT];
+	dst_height = tot_height - intel_crtc->border[PANEL_BORDER_TOP] -
+				intel_crtc->border[PANEL_BORDER_BOTTOM];
+
+	if ((dst_width == 0) || (dst_height == 0)) {
+		DRM_ERROR("Invalid border size input\n");
+		return;
+	}
+
+	pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
+	pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
+
+	if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("width is too small\n");
+		return;
+	} else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
+		DRM_ERROR("height is too small\n");
+		return;
+	}
+
+	if (dst_width != tot_width)
+		apply_horizontal_borders(adjusted_mode, intel_crtc->border);
+	if (dst_height != tot_height)
+		apply_vertical_borders(adjusted_mode, intel_crtc->border);
+
+	/* No scaling needed now, but still enable the panel fitter,
+	   as that will allow the User to subequently do the dynamic
+	   flipping of fbs of different resolutions */
+	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) {
+		DRM_DEBUG_KMS("Forcefully enabling the Panel fitter\n");
+	}
+
+	border = LVDS_BORDER_ENABLE;
+
+	if (INTEL_INFO(dev)->gen >= 4) {
+		/* PFIT_SCALING_PROGRAMMED is de-featured on BYT */
+		pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
+		pfit_control |= ((intel_crtc->pipe << PFIT_PIPE_SHIFT) | PFIT_FILTER_FUZZY);
+	} else {
+		pfit_control |= (PFIT_ENABLE |
+				 VERT_AUTO_SCALE | HORIZ_AUTO_SCALE |
+				 VERT_INTERP_BILINEAR | HORIZ_INTERP_BILINEAR);
+	}
+
+	/* Make sure pre-965 set dither correctly for 18bpp panels. */
+	if (INTEL_INFO(dev)->gen < 4 && pipe_config->pipe_bpp == 18)
+		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
+
+	pipe_config->gmch_pfit.control = pfit_control;
+	pipe_config->gmch_pfit.lvds_border_bits = border;
+}
+
 void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			      struct intel_crtc_config *pipe_config,
 			      int fitting_mode)
@@ -257,6 +442,12 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 
 	adjusted_mode = &pipe_config->adjusted_mode;
 
+	/* check if User wants to apply the borders, otherwise fall
+	   through the regular path */
+	if (intel_crtc->apply_borders)
+		return intel_gmch_manual_panel_fitting(intel_crtc,
+						       pipe_config);
+
 	/* Native modes don't need fitting */
 	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2014-04-20 10:46 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-10  5:10 [PATCH 0/3] Two new drm crtc properties akash.goel
2014-03-10  5:10 ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
2014-03-10  5:10 ` [PATCH 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
2014-03-10  5:10 ` [PATCH 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
2014-03-10  5:14 ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
2014-03-10  5:17   ` Goel, Akash
2014-03-11 12:54   ` [PATCH 1/3] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure akash.goel
2014-03-11 12:54     ` [PATCH v2 0/3] Two new drm crtc properties akash.goel
2014-03-21 19:15       ` Goel, Akash
2014-03-11 12:54     ` [PATCH v2 2/3] drm/i915: New drm crtc property for varying the Pipe Src size akash.goel
2014-03-11 12:54     ` [PATCH v2 3/3] drm/i915: New drm crtc property for varying the size of borders akash.goel
2014-03-23  8:56       ` [PATCH v3] " akash.goel
2014-03-26  3:55         ` [PATCH v4 3/3] " akash.goel
2014-04-07  4:06           ` Goel, Akash
2014-04-08 16:31             ` Damien Lespiau
2014-04-08 16:32               ` Damien Lespiau
2014-04-08 16:28           ` Ville Syrjälä
2014-04-10  7:43             ` Akash Goel
2014-04-10 11:34               ` Ville Syrjälä
2014-04-11  3:04                 ` Akash Goel
2014-04-11 11:42                   ` Ville Syrjälä
2014-04-13 12:28                     ` Akash Goel
2014-04-13 17:12                       ` Daniel Vetter
2014-04-20 10:49                     ` [PATCH v5 3/4] " akash.goel
2014-03-21 19:53   ` [PATCH 0/3] Two new drm crtc properties Daniel Vetter
2014-03-22  4:29     ` Goel, Akash

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.