diff mbox

[v4,3/3] drm/i915: New drm crtc property for varying the size of borders

Message ID 1395806112-21713-1-git-send-email-akash.goel@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

akash.goel@intel.com March 26, 2014, 3:55 a.m. UTC
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(-)

Comments

Ville Syrjälä April 8, 2014, 4:28 p.m. UTC | #1
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
akash.goel@intel.com April 10, 2014, 7:43 a.m. UTC | #2
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
>
Ville Syrjälä April 10, 2014, 11:34 a.m. UTC | #3
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
> > 
>
akash.goel@intel.com April 11, 2014, 3:04 a.m. UTC | #4
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
> > > 
> > 
>
Ville Syrjälä April 11, 2014, 11:42 a.m. UTC | #5
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
> > > > 
> > > 
> > 
>
akash.goel@intel.com April 13, 2014, 12:28 p.m. UTC | #6
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
> > > > > 
> > > > 
> > > 
> > 
>
Daniel Vetter April 13, 2014, 5:12 p.m. UTC | #7
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
diff mbox

Patch

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)