diff mbox

[11/14] drm/mgag200: Consolidate depth/bpp handling

Message ID 20170718144320.8354-12-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai July 18, 2017, 2:43 p.m. UTC
From: Egbert Eich <eich@suse.de>

The depth/bpp handling for chips with limited memory in commit
918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
16-bit") was incomplete: the bpp limits were applied to mode
validation.

This consolidates dpeth/bpp handling, adds it to mode validation
and moves the code which reads the command line specified depth
into the correct location.

Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")
Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
 drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
 4 files changed, 16 insertions(+), 16 deletions(-)

Comments

Paul Menzel July 20, 2017, 11:58 a.m. UTC | #1
Dear Takashi,


Thank you for posting these patches for review.


Am Dienstag, den 18.07.2017, 16:43 +0200 schrieb Takashi Iwai:
> From: Egbert Eich <eich@suse.de>
> 
> The depth/bpp handling for chips with limited memory in commit
> 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
> 16-bit") was incomplete: the bpp limits were applied to mode
> validation.
> 
> This consolidates dpeth/bpp handling, adds it to mode validation

depth

> and moves the code which reads the command line specified depth
> into the correct location.
> 
> Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")

```
$ git tag --contains 918be888d613 | head -1
v3.14
```

Please mark this as stable then too?

Also, could the original commit author time-stamps be preserved if you
have them in your SUSE repositories? The `Date` line could be added
below the `From` line. That would help to know for how long the changes
have been tested.

> Signed-off-by: Egbert Eich <eich@suse.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
>  drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
>  drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
>  4 files changed, 16 insertions(+), 16 deletions(-)

[…]

Otherwise this looks fine.


Thanks,

Paul
Takashi Iwai July 20, 2017, 12:08 p.m. UTC | #2
On Thu, 20 Jul 2017 13:58:14 +0200,
Paul Menzel wrote:
> 
> Dear Takashi,
> 
> 
> Thank you for posting these patches for review.
> 
> 
> Am Dienstag, den 18.07.2017, 16:43 +0200 schrieb Takashi Iwai:
> > From: Egbert Eich <eich@suse.de>
> > 
> > The depth/bpp handling for chips with limited memory in commit
> > 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
> > 16-bit") was incomplete: the bpp limits were applied to mode
> > validation.
> > 
> > This consolidates dpeth/bpp handling, adds it to mode validation
> 
> depth
> 
> > and moves the code which reads the command line specified depth
> > into the correct location.
> > 
> > Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")
> 
> ```
> $ git tag --contains 918be888d613 | head -1
> v3.14
> ```
> 
> Please mark this as stable then too?

I guess it deserves for stable, yes.
I leave the decision to the maintainer.

> Also, could the original commit author time-stamps be preserved if you
> have them in your SUSE repositories? The `Date` line could be added
> below the `From` line. That would help to know for how long the changes
> have been tested.

Unfortunately, we don't know how old the original patch was.  The date
isn't preserved in the original patch, either, as it's stored in quilt
queue, not committed in a kernel git tree.

In general, the original date isn't that important, so let's skip that
part.

> 
> > Signed-off-by: Egbert Eich <eich@suse.de>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
> >  drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
> >  drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
> >  drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
> >  4 files changed, 16 insertions(+), 16 deletions(-)
> 
> […]
> 
> Otherwise this looks fine.

Thanks for your review!


Takashi
diff mbox

Patch

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 1ba559c93b8f..9d7ae61ce915 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -209,6 +209,8 @@  struct mga_device {
 	int				has_sdram;
 	struct drm_display_mode		mode;
 
+	int preferred_bpp;
+
 	int bpp_shifts[4];
 
 	int fb_mtrr;
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index bb9a4c409dc0..173aed918183 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -267,11 +267,6 @@  int mgag200_fbdev_init(struct mga_device *mdev)
 {
 	struct mga_fbdev *mfbdev;
 	int ret;
-	int bpp_sel = 32;
-
-	/* prefer 16bpp on low end gpus with limited VRAM */
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
-		bpp_sel = 16;
 
 	mfbdev = devm_kzalloc(mdev->dev->dev, sizeof(struct mga_fbdev), GFP_KERNEL);
 	if (!mfbdev)
@@ -294,7 +289,7 @@  int mgag200_fbdev_init(struct mga_device *mdev)
 	/* disable all the possible outputs/crtcs before entering KMS mode */
 	drm_helper_disable_unused_functions(mdev->dev);
 
-	ret = drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel);
+	ret = drm_fb_helper_initial_config(&mfbdev->helper, mdev->preferred_bpp);
 	if (ret)
 		goto err_fb_setup;
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 73614154a5ef..2ec76d6615a8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -339,10 +339,13 @@  int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = (void *)&mga_mode_funcs;
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
-		dev->mode_config.preferred_depth = 16;
-	else
+	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024)) {
+		/* prefer 16bpp on low end gpus with limited VRAM */
+		mdev->preferred_bpp = dev->mode_config.preferred_depth = 16;
+	} else {
+		mdev->preferred_bpp = 32;
 		dev->mode_config.preferred_depth = 24;
+	}
 	dev->mode_config.prefer_shadow = 1;
 
 	r = mgag200_modeset_init(mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index a07f67ed6e4a..77b8efcd3c65 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1705,9 +1705,15 @@  static int mga_vga_mode_valid(struct drm_connector *connector,
 {
 	struct drm_device *dev = connector->dev;
 	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
-	int bpp = 32;
+	int bpp;
 	int lace;
 
+	bpp = mdev->preferred_bpp;
+	/* Validate the mode input by the user */
+	if (connector->cmdline_mode.specified &&
+	    connector->cmdline_mode.bpp_specified)
+		bpp = connector->cmdline_mode.bpp;
+
 	lace = (mode->flags & DRM_MODE_FLAG_INTERLACE) ? 2 : 1;
 	if (IS_G200_SE(mdev)) {
 		if (mdev->unique_rev_id == 0x01) {
@@ -1767,12 +1773,6 @@  static int mga_vga_mode_valid(struct drm_connector *connector,
 		return MODE_BAD;
 	}
 
-	/* Validate the mode input by the user */
-	if (connector->cmdline_mode.specified) {
-		if (connector->cmdline_mode.bpp_specified)
-			bpp = connector->cmdline_mode.bpp;
-	}
-
 	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
 		if (connector->cmdline_mode.specified)
 			connector->cmdline_mode.specified = false;