Re: [PATCH] drm/mgag200: Reject modes that are too big for VRAM

From: Paul Menzel <paulepanter@users.sourceforge.net>
To: Christopher Harvey <charvey@matrox.com>
Cc: Julia Lemire <jlemire@matrox.com>,
	Mathieu Larouche <mathieu.larouche@matrox.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/mgag200: Reject modes that are too big for VRAM
Date: Wed, 27 Feb 2013 00:34:56 +0100	[thread overview]
Message-ID: <1361921696.6140.34.camel@mattotaupa> (raw)
In-Reply-To: <20130226155544.GA18661@harvey-pc.matrox.com>

[-- Attachment #1.1: Type: text/plain, Size: 2605 bytes --]

Am Dienstag, den 26.02.2013, 10:55 -0500 schrieb Christopher Harvey:
> A monitor or a user could request a resolution greater than the
> available VRAM for the backing framebuffer. This change checks the
> required framebuffer size against the max VRAM size and rejects modes
> if they are too big. This change can also remove a mode request passed
> in via the video= parameter.
> 
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 3abf197..6b5db83 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1405,6 +1405,14 @@ static int mga_vga_get_modes(struct drm_connector *connector)
>  static int mga_vga_mode_valid(struct drm_connector *connector,
>  				 struct drm_display_mode *mode)
>  {
> +	struct drm_device *dev = connector->dev;
> +	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
> +	struct mga_fbdev *mfbdev = mdev->mfbdev;
> +	struct drm_fb_helper *fb_helper = &mfbdev->helper;
> +	struct drm_fb_helper_connector *fb_helper_conn = NULL;
> +	int bpp = 32;
> +	int i = 0;

It is initialized in the for loop again.

> +
>  	/* FIXME: Add bandwidth and g200se limitations */
>  
>  	if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 ||
> @@ -1414,6 +1422,25 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
>  		return MODE_BAD;
>  	}
>  
> +	/* Validate the mode input by the user */
> +	for (i = 0; i < fb_helper->connector_count; i++) {
> +		if (fb_helper->connector_info[i]->connector == connector) {
> +			/* Found the helper for this connector */
> +			fb_helper_conn = fb_helper->connector_info[i];
> +			if (fb_helper_conn->cmdline_mode.specified) {
> +				if (fb_helper_conn->cmdline_mode.bpp_specified) {
> +					bpp = fb_helper_conn->cmdline_mode.bpp;
> +				}
> +			}
> +		}
> +	}

Is such a function not used somewhere else already? Like get
user_parameters or so?

> +
> +	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
> +		if (fb_helper_conn)
> +			fb_helper_conn->cmdline_mode.specified = false;

A debug message specifying that this is due to VRAM size would be
helpful I guess.

> +		return MODE_BAD;

I guess that will print the requested mode so it does not need to be
specified above.

> +	}
> +
>  	return MODE_OK;
>  }
>  

Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel