On Fri, Mar 07, 2014 at 09:08:40AM +0000, Goel, Akash wrote:
> Hi Ville,
> 
> Please review this patch. This is just a provisional patch. 
> Here we have tried to extend the 'scaling mode' property by adding a new 
> 'manual' mode and User will have to specify the display window (Panel 
> fitter's output) through another property. But this may not be enough.

The panel fitter source size property should be on the crtc, not the
connector.

> 
> To support the arbitrary use of Panel fitter, as per your & Chris's 
> suggestions, we can use the following 2 approaches. 
> 
> 1. Introduce a new property (of blob type) through which a structure would be 
> passed, which will have the Panel fitter input (PIPESRC) & output info. Using 
> this Driver can then do a modeset internally.
>      'fb_id' info will also be required to passed here, conforming to the 
> PIPESRC. 

No, fb_id is passed through setcrtc (or soon through setplane, I hope).

> Or 
> 2. Add the border info to the display mode structure which will control the 
> Panel fitter output. The panel fitter input (i.e. PIPESRC) will be provided 
> through the regular 'hdisplay',  'vdisplay' fields.

No, we need both the border and the PIPESRC information.
hdisplay/vdisplays isn't enough since the user facing mode structure
doesn't have vblank_start/end fields. Hence we can't specify borders.

If we don't want to extend the display mode, we should add border properties
of some sort. I know some drivers have added something like that to the
connectors, but again since the mode is applied to the crtc, ideally these
properties should also be on the crtc.

>      This way the PIPESRC programing & Panel fitter mode update could be done 
> in a single modeset call.

This is a separate issue. We shouldn't add kludges for it since it will
get solved properly by the atomic modeset ioctl.

> 
> In both the cases, we can have an additional check that if only the Panel 
> fitter input (fb size) is getting changed & Panel fitter reprogramming is not 
> required, then a full modeset can be avoided except the PIPESRC programming.  
> This could allow to dynamically flip the frame buffers of different 
> resolution without a modeset.

Checks like that need to happen at the modeset compute stage, so we can
optimize away all the case. We shouldn't sprinkle special cases here and
there.

> 
> Best regards
> Akash
> 
> -----Original Message-----
> From: Goel, Akash 
> Sent: Tuesday, March 04, 2014 3:42 PM
> To: [email protected]
> Cc: Goel, Akash; G, Pallavi
> Subject: [RFC] drm/i915 : Added a new 'window size' property for connector
> 
> From: Akash Goel <[email protected]>
> 
> Added a new 'window size' property for connectors.
> This can be used to control the output window size of Panel fitter.
> Also addd a new mode 'Manual' to existing 'scaling mode'
> property.
> 
> Signed-off-by: G Pallavi <[email protected]>
> Signed-off-by: Akash Goel <[email protected]>
> ---
>  drivers/gpu/drm/drm_crtc.c         |   1 +
>  drivers/gpu/drm/i915/intel_dp.c    |  58 ++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h   |   8 +++
>  drivers/gpu/drm/i915/intel_panel.c | 104 
> +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h             |   3 ++
>  include/uapi/drm/drm_mode.h        |   1 +
>  6 files changed, 167 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 
> 35ea15d..1d237b5 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -123,6 +123,7 @@ static const struct drm_prop_enum_list 
> drm_scaling_mode_enum_list[] =
>       { DRM_MODE_SCALE_FULLSCREEN, "Full" },
>       { DRM_MODE_SCALE_CENTER, "Center" },
>       { DRM_MODE_SCALE_ASPECT, "Full aspect" },
> +     { DRM_MODE_SCALE_MANUAL, "Manual" },
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> b/drivers/gpu/drm/i915/intel_dp.c index c512d78..c697b7a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -886,12 +886,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>       if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>               intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>                                      adjusted_mode);
> -             if (!HAS_PCH_SPLIT(dev))
> -                     intel_gmch_panel_fitting(intel_crtc, pipe_config,
> -                                              
> intel_connector->panel.fitting_mode);
> -             else
> -                     intel_pch_panel_fitting(intel_crtc, pipe_config,
> -                                             
> intel_connector->panel.fitting_mode);
> +             if (!HAS_PCH_SPLIT(dev)) {
> +                     if (intel_connector->panel.fitting_mode == 
> DRM_MODE_SCALE_MANUAL)
> +                             intel_gmch_manual_panel_fitting(intel_crtc, 
> pipe_config,
> +                                                     
> intel_connector->panel.display_window_size);
> +                     else
> +                             intel_gmch_panel_fitting(intel_crtc, 
> pipe_config,
> +                                                      
> intel_connector->panel.fitting_mode);
> +             }
> +             else {
> +                     if (intel_connector->panel.fitting_mode == 
> DRM_MODE_SCALE_MANUAL)
> +                             intel_pch_manual_panel_fitting(intel_crtc, 
> pipe_config,
> +                                                     
> intel_connector->panel.display_window_size);
> +                     else
> +                             intel_pch_panel_fitting(intel_crtc, pipe_config,
> +                                                     
> intel_connector->panel.fitting_mode);
> +             }
>       }
>  
>       if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) @@ -3376,14 +3386,36 
> @@ intel_dp_set_property(struct drm_connector *connector,
>               }
>  
>               if (intel_connector->panel.fitting_mode == val) {
> -                     /* the eDP scaling property is not changed */
> -                     return 0;
> +                     /* Check if display window size has changed */
> +                     if ((val == DRM_MODE_SCALE_MANUAL) &&
> +                          intel_connector->panel.display_window_size_changed)
> +                             
> intel_connector->panel.display_window_size_changed = 0;
> +                     else {
> +                             /* the eDP scaling property is not changed */
> +                             return 0;
> +                     }
>               }
>               intel_connector->panel.fitting_mode = val;
>  
> +             if (val != DRM_MODE_SCALE_MANUAL)
> +                     intel_connector->panel.display_window_size = 0;
> +
>               goto done;
>       }
>  
> +     if (is_edp(intel_dp) &&
> +         property == connector->dev->mode_config.window_size_property) {
> +             if (intel_connector->panel.display_window_size == val) {
> +                     /* the eDP panel window size property is not changed */
> +                     intel_connector->panel.display_window_size_changed = 0;
> +                     return 0;
> +             }
> +             intel_connector->panel.display_window_size = val;
> +             intel_connector->panel.display_window_size_changed = 1;
> +
> +             return 0;
> +     }
> +
>       return -EINVAL;
>  
>  done:
> @@ -3517,6 +3549,16 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
> struct drm_connector *connect
>                       connector->dev->mode_config.scaling_mode_property,
>                       DRM_MODE_SCALE_ASPECT);
>               intel_connector->panel.fitting_mode = DRM_MODE_SCALE_ASPECT;
> +
> +             connector->dev->mode_config.window_size_property =
> +                     drm_property_create_range(connector->dev, 0,
> +                                             "window size", 0, 0xFFFFFFFF);
> +             drm_object_attach_property(
> +                     &connector->base,
> +                     connector->dev->mode_config.window_size_property,
> +                     0);
> +             intel_connector->panel.display_window_size = 0;
> +             intel_connector->panel.display_window_size_changed = 0;
>       }
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index a4ffc02..5247b6b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -157,6 +157,8 @@ struct intel_panel {
>       struct drm_display_mode *fixed_mode;
>       struct drm_display_mode *downclock_mode;
>       int fitting_mode;
> +     u32 display_window_size;
> +     bool display_window_size_changed;
>  
>       /* backlight */
>       struct {
> @@ -841,9 +843,15 @@ void intel_fixed_panel_mode(const struct 
> drm_display_mode *fixed_mode,  void intel_pch_panel_fitting(struct intel_crtc 
> *crtc,
>                            struct intel_crtc_config *pipe_config,
>                            int fitting_mode);
> +void intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> +                                 struct intel_crtc_config *pipe_config,
> +                                 u32 display_window_size);
>  void intel_gmch_panel_fitting(struct intel_crtc *crtc,
>                             struct intel_crtc_config *pipe_config,
>                             int fitting_mode);
> +void intel_gmch_manual_panel_fitting(struct intel_crtc *crtc,
> +                           struct intel_crtc_config *pipe_config,
> +                           u32 display_window_size);
>  void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
>                              u32 max);
>  int intel_panel_setup_backlight(struct drm_connector *connector); diff --git 
> a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index cb05840..2965975 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -114,6 +114,48 @@ done:
>       pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;  }
>  
> +void
> +intel_pch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> +                     struct intel_crtc_config *pipe_config,
> +                     u32 display_window_size)
> +{
> +     struct drm_display_mode *adjusted_mode;
> +     int x, y;
> +     u32 tot_width, tot_height;
> +     u32 dst_width, dst_height;
> +
> +     adjusted_mode = &pipe_config->adjusted_mode;
> +
> +     tot_width  = adjusted_mode->crtc_hdisplay;
> +     tot_height = adjusted_mode->crtc_vdisplay;
> +
> +     dst_width  = ((display_window_size >> 16) & 0xffff);
> +     dst_height = (display_window_size & 0xffff);
> +
> +     x = y = 0;
> +
> +     if (tot_width < dst_width) {
> +             DRM_ERROR("width is too big\n");
> +             return;
> +     } else if (dst_width & 1) {
> +             DRM_ERROR("width must be even\n");
> +             return;
> +     } else if (tot_height < dst_height) {
> +             DRM_ERROR("height is too big\n");
> +             return;
> +     } else if (dst_height & 1) {
> +             DRM_ERROR("height must be even\n");
> +             return;
> +     }
> +
> +     x = (adjusted_mode->hdisplay - dst_width + 1)/2;
> +     y = (adjusted_mode->vdisplay - dst_height + 1)/2;
> +
> +     pipe_config->pch_pfit.pos = (x << 16) | y;
> +     pipe_config->pch_pfit.size = (dst_width << 16) | dst_height;
> +     pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0; }
> +
>  static void
>  centre_horizontally(struct drm_display_mode *mode,
>                   int width)
> @@ -323,6 +365,68 @@ out:
>       pipe_config->gmch_pfit.lvds_border_bits = border;  }
>  
> +void intel_gmch_manual_panel_fitting(struct intel_crtc *intel_crtc,
> +                                  struct intel_crtc_config *pipe_config,
> +                                  u32 display_window_size)
> +{
> +     u32 pfit_control = 0, border = 0;
> +     u32 pf_horizontal_ratio, pf_vertical_ratio;
> +     struct drm_display_mode *adjusted_mode;
> +     u32 tot_width, tot_height;
> +     u32 src_width, src_height; /* pipesrc.x, pipesrc.y */
> +     u32 dst_width, dst_height;
> +
> +     adjusted_mode = &pipe_config->adjusted_mode;
> +
> +     src_width = pipe_config->pipe_src_w;
> +     src_height = pipe_config->pipe_src_h;
> +
> +     tot_width  = adjusted_mode->crtc_hdisplay;
> +     tot_height = adjusted_mode->crtc_vdisplay;
> +
> +     dst_width  = ((display_window_size >> 16) & 0xffff);
> +     dst_height = (display_window_size & 0xffff);
> +
> +     pf_horizontal_ratio = panel_fitter_scaling(src_width, dst_width);
> +     pf_vertical_ratio   = panel_fitter_scaling(src_height, dst_height);
> +
> +/* Max Downscale ratio of 1.125, expressed in 1.12 fixed point format 
> +*/ #define MAX_DOWNSCALE_RATIO  (0x9 << 9)
> +
> +     if (pf_horizontal_ratio > MAX_DOWNSCALE_RATIO) {
> +                     DRM_ERROR("width is too small\n");
> +                     return;
> +     } else if (tot_width < dst_width) {
> +                     DRM_ERROR("width is too big\n");
> +                     return;
> +     } else if (dst_width & 1) {
> +                     DRM_ERROR("width must be even\n");
> +                     return;
> +     } else if (pf_vertical_ratio > MAX_DOWNSCALE_RATIO) {
> +                     DRM_ERROR("height is too small\n");
> +                     return;
> +     } else if (tot_height < dst_height) {
> +                     DRM_ERROR("height is too big\n");
> +                     return;
> +     } else if (dst_height & 1) {
> +                     DRM_ERROR("height must be even\n");
> +                     return;
> +     }
> +
> +     centre_horizontally(adjusted_mode, dst_width);
> +     centre_vertically(adjusted_mode, dst_height);
> +     border = LVDS_BORDER_ENABLE;
> +
> +     /* 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);
> +
> +     pipe_config->gmch_pfit.control = pfit_control;
> +     pipe_config->gmch_pfit.lvds_border_bits = border;
> +
> +     return;
> +}
> +
>  static u32 intel_panel_compute_brightness(struct intel_connector *connector,
>                                         u32 val)
>  {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 
> f764654..625d7fd 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -902,6 +902,9 @@ struct drm_mode_config {
>       struct drm_property *scaling_mode_property;
>       struct drm_property *dirty_info_property;
>  
> +     /* Panel fitter output property */
> +     struct drm_property *window_size_property;
> +
>       /* dumb ioctl parameters */
>       uint32_t preferred_depth, prefer_shadow;
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 
> f104c26..a187dac 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -87,6 +87,7 @@
>  #define DRM_MODE_SCALE_FULLSCREEN    1 /* Full screen, ignore aspect */
>  #define DRM_MODE_SCALE_CENTER                2 /* Centered, no scaling */
>  #define DRM_MODE_SCALE_ASPECT                3 /* Full screen, preserve 
> aspect */
> +#define DRM_MODE_SCALE_MANUAL                4 /* User can control display 
> window size */
>  
>  /* Dithering mode options */
>  #define DRM_MODE_DITHERING_OFF       0
> --
> 1.8.5.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to