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@xxxxxxxxx wrote:
> > > From: Akash Goel <akash.goel@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
