On Tue, Oct 01, 2013 at 04:13:31PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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ä <ville.syrjala@xxxxxxxxxxxxxxx>
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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
