On Tue, Oct 01, 2013 at 04:13:31PM +0300, [email protected] wrote:
> From: Ville Syrjälä <[email protected]>
> 
> Make sure the active, blanking and sync portions of the timings are in
> the proper order. The contract with userspace basically is that we must
> not increase the active portion. For the rest we're more or less free to
> do as we please. A good rul IMO is that we only increase the non-active
> portions of the timings.
> 
> We could do a lot more adjustment to make sure the timings meet all
> the minimum requirements of the hardware. But as the mimimums may depend
> on the output type and other details, we should perhaps do the adjustments
> at some later point.
> 
> Signed-off-by: Ville Syrjälä <[email protected]>

Do we actually get such modes?

I'd vote to just filter them from our ->get_modes callbacks and reject
them here if at all possible ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 60 
> ++++++++++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 7f61cfb..48cad99 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8458,6 +8458,55 @@ static bool check_encoder_cloning(struct drm_crtc 
> *crtc)
>       return !(num_encoders > 1 && uncloneable_encoders);
>  }
>  
> +static void adjust_timings(struct drm_display_mode *mode,
> +                        int *val, int min, int clocks_per_unit)
> +{
> +     if (*val >= min)
> +             return;
> +
> +     /* maintain the same refresh rate */
> +     mode->clock += (min - *val) * clocks_per_unit;
> +
> +     *val = min;
> +}
> +
> +static int intel_adjust_mode(struct drm_display_mode *mode)
> +{
> +     if (mode->clock == 0) {
> +             DRM_DEBUG_KMS("bad pixel clock\n");
> +             drm_mode_debug_printmodeline(mode);
> +             return -EINVAL;
> +     }
> +
> +     if (mode->hdisplay == 0 || mode->vdisplay == 0) {
> +             DRM_DEBUG_KMS("bad hdisplay or vdisplay\n");
> +             drm_mode_debug_printmodeline(mode);
> +             return -EINVAL;
> +     }
> +
> +     adjust_timings(mode, &mode->hsync_start, mode->hdisplay, 0);
> +     adjust_timings(mode, &mode->hsync_end, mode->hsync_start, 0);
> +     adjust_timings(mode, &mode->htotal, mode->hsync_end, mode->vtotal);
> +
> +     adjust_timings(mode, &mode->vsync_start, mode->vdisplay, 0);
> +     adjust_timings(mode, &mode->vsync_end, mode->vsync_start, 0);
> +
> +     /*
> +      * We must not increase the size of the active
> +      * space area for frame packing modes.
> +      */
> +     if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == 
> DRM_MODE_FLAG_3D_FRAME_PACKING) {
> +             if (mode->vtotal <  mode->vsync_end) {
> +                     DRM_DEBUG_KMS("refusing to adjust vtotal for frame 
> packing mode\n");
> +                     drm_mode_debug_printmodeline(mode);
> +                     return -EINVAL;
> +             }
> +     } else
> +             adjust_timings(mode, &mode->vtotal, mode->vsync_end, 
> mode->htotal);
> +
> +     return 0;
> +}
> +
>  static struct intel_crtc_config *
>  intel_modeset_pipe_config(struct drm_crtc *crtc,
>                         struct drm_framebuffer *fb,
> @@ -8466,7 +8515,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>       struct drm_device *dev = crtc->dev;
>       struct intel_encoder *encoder;
>       struct intel_crtc_config *pipe_config;
> -     int plane_bpp, ret = -EINVAL;
> +     int plane_bpp, ret;
>       bool retry = true;
>  
>       if (!check_encoder_cloning(crtc)) {
> @@ -8498,14 +8547,20 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>             (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
>               pipe_config->adjusted_mode.flags |= DRM_MODE_FLAG_NVSYNC;
>  
> +     ret = intel_adjust_mode(&pipe_config->adjusted_mode);
> +     if (ret)
> +             goto fail;
> +
>       /* Compute a starting value for pipe_config->pipe_bpp taking the source
>        * plane pixel format and any sink constraints into account. Returns the
>        * source plane bpp so that dithering can be selected on mismatches
>        * after encoders and crtc also have had their say. */
>       plane_bpp = compute_baseline_pipe_bpp(to_intel_crtc(crtc),
>                                             fb, pipe_config);
> -     if (plane_bpp < 0)
> +     if (plane_bpp < 0) {
> +             ret = -EINVAL;
>               goto fail;
> +     }
>  
>       /* Determine the real pipe dimensions */
>       drm_mode_set_crtcinfo(&pipe_config->adjusted_mode, CRTC_STEREO_DOUBLE);
> @@ -8532,6 +8587,7 @@ encoder_retry:
>  
>               if (!(encoder->compute_config(encoder, pipe_config))) {
>                       DRM_DEBUG_KMS("Encoder config failure\n");
> +                     ret = -EINVAL;
>                       goto fail;
>               }
>       }
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to