When the mode is set with 16bpp on QEMU, the output gets totally broken. The culprit is the bogus register values set for 16bpp, which was likely copied from from a wrong place.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=799216
Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/gpu/drm/cirrus/cirrus_mode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index 60685b2..379a47e 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -273,8 +273,8 @@ static int cirrus_crtc_mode_set(struct drm_crtc *crtc, sr07 |= 0x11; break; case 16: - sr07 |= 0xc1; - hdr = 0xc0; + sr07 |= 0x17; + hdr = 0xc1; break; case 24: sr07 |= 0x15;
We've got a bug report that GNOME on QEMU shows wrong colors. It turned out that it's because Cairo doesn't support 24bpp well. This hasn't been an issue until now because we (at least SUSE and Fedora) have a patch to use 16bpp for QEMU in Xorg cirrus UMS driver.
Since cirrus KMS driver is mainly targeted for the use on QEMU/KVM, we should choose 16bpp as default, too.
Also, it's not convenient to set the default bpp in multiple places. cirrus_fbdev_init() should check the original preferred depth set in cirrus_modeset_init().
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=799216
Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 +- drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 6c6b4c8..bc081ea 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -273,7 +273,7 @@ int cirrus_fbdev_init(struct cirrus_device *cdev) { struct cirrus_fbdev *gfbdev; int ret; - int bpp_sel = 24; + int bpp_sel = cdev->dev->mode_config.preferred_depth;
/*bpp_sel = 8;*/ gfbdev = kzalloc(sizeof(struct cirrus_fbdev), GFP_KERNEL); diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index 379a47e..e259f07 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -588,7 +588,7 @@ int cirrus_modeset_init(struct cirrus_device *cdev) cdev->dev->mode_config.max_height = CIRRUS_MAX_FB_HEIGHT;
cdev->dev->mode_config.fb_base = cdev->mc.vram_base; - cdev->dev->mode_config.preferred_depth = 24; + cdev->dev->mode_config.preferred_depth = 16; /* don't prefer a shadow on virt GPU */ cdev->dev->mode_config.prefer_shadow = 0;
Add a new option, bpp, to specify the default bpp value.
Signed-off-by: Takashi Iwai tiwai@suse.de ---
This patch is applied on the top of previous two patches. I couldn't find an easy way to specify the default bpp, so I cooked the driver quickly. If there is any other convenient way to achieve this, let me know...
Takashi
=== drivers/gpu/drm/cirrus/cirrus_drv.c | 9 +++++++++ drivers/gpu/drm/cirrus/cirrus_drv.h | 2 ++ drivers/gpu/drm/cirrus/cirrus_mode.c | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c b/drivers/gpu/drm/cirrus/cirrus_drv.c index 8ecb601..407750fb 100644 --- a/drivers/gpu/drm/cirrus/cirrus_drv.c +++ b/drivers/gpu/drm/cirrus/cirrus_drv.c @@ -15,9 +15,12 @@ #include "cirrus_drv.h"
int cirrus_modeset = -1; +int cirrus_bpp = 16;
MODULE_PARM_DESC(modeset, "Disable/Enable modesetting"); module_param_named(modeset, cirrus_modeset, int, 0400); +MODULE_PARM_DESC(bpp, "Default bits per pixel"); +module_param_named(bpp, cirrus_bpp, int, 0400);
/* * This is the generic driver code. This binds the driver to the drm core, @@ -121,6 +124,12 @@ static int __init cirrus_init(void)
if (cirrus_modeset == 0) return -EINVAL; + + if (cirrus_bpp % 8 || cirrus_bpp < 8 || cirrus_bpp > 24) { + pr_err("cirrus: invalid bpp %d, default to 16\n", cirrus_bpp); + cirrus_bpp = 16; + } + return drm_pci_init(&driver, &cirrus_pci_driver); }
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h index 6e0cc72..45ffdb8 100644 --- a/drivers/gpu/drm/cirrus/cirrus_drv.h +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h @@ -176,6 +176,8 @@ cirrus_bo(struct ttm_buffer_object *bo) #define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base) #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
+extern int cirrus_modeset; +extern int cirrus_bpp; /* cirrus_mode.c */ void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green, u16 blue, int regno); diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index e259f07..3524081 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -588,7 +588,7 @@ int cirrus_modeset_init(struct cirrus_device *cdev) cdev->dev->mode_config.max_height = CIRRUS_MAX_FB_HEIGHT;
cdev->dev->mode_config.fb_base = cdev->mc.vram_base; - cdev->dev->mode_config.preferred_depth = 16; + cdev->dev->mode_config.preferred_depth = cirrus_bpp; /* don't prefer a shadow on virt GPU */ cdev->dev->mode_config.prefer_shadow = 0;
On Tue, Jan 29, 2013 at 09:29:17AM +0100, Takashi Iwai wrote:
Add a new option, bpp, to specify the default bpp value.
Signed-off-by: Takashi Iwai tiwai@suse.de
This patch is applied on the top of previous two patches. I couldn't find an easy way to specify the default bpp, so I cooked the driver quickly. If there is any other convenient way to achieve this, let me know...
Well, you can specify the desired bpp with a full mode on the kernel cmdline - the '-bpp' extension. Reading through the parser I think it should work even with just the '-bpp' and not a full mode, but I haven't tested. Look for cmdline_mode->bpp_specified in drm_fb_helper.c and the relevant parsing code in drm_mode_parse_command_line_for_connector in drm_modes.c
If that doesn't work for you, I think it's better to extend/fix it than add driver module options.
Cheers, Daniel
At Tue, 29 Jan 2013 10:53:50 +0100, Daniel Vetter wrote:
On Tue, Jan 29, 2013 at 09:29:17AM +0100, Takashi Iwai wrote:
Add a new option, bpp, to specify the default bpp value.
Signed-off-by: Takashi Iwai tiwai@suse.de
This patch is applied on the top of previous two patches. I couldn't find an easy way to specify the default bpp, so I cooked the driver quickly. If there is any other convenient way to achieve this, let me know...
Well, you can specify the desired bpp with a full mode on the kernel cmdline - the '-bpp' extension. Reading through the parser I think it should work even with just the '-bpp' and not a full mode, but I haven't tested. Look for cmdline_mode->bpp_specified in drm_fb_helper.c and the relevant parsing code in drm_mode_parse_command_line_for_connector in drm_modes.c
If that doesn't work for you, I think it's better to extend/fix it than add driver module options.
Well, the fb can be set by that option, but the default modeset doesn't seem to honor it. So if you start X modeset driver, it still takes what the driver sets as default. That was the problem I hit.
thanks,
Takashi
On Tue, Jan 29, 2013 at 10:57 AM, Takashi Iwai tiwai@suse.de wrote:
At Tue, 29 Jan 2013 10:53:50 +0100, Daniel Vetter wrote:
On Tue, Jan 29, 2013 at 09:29:17AM +0100, Takashi Iwai wrote:
Add a new option, bpp, to specify the default bpp value.
Signed-off-by: Takashi Iwai tiwai@suse.de
This patch is applied on the top of previous two patches. I couldn't find an easy way to specify the default bpp, so I cooked the driver quickly. If there is any other convenient way to achieve this, let me know...
Well, you can specify the desired bpp with a full mode on the kernel cmdline - the '-bpp' extension. Reading through the parser I think it should work even with just the '-bpp' and not a full mode, but I haven't tested. Look for cmdline_mode->bpp_specified in drm_fb_helper.c and the relevant parsing code in drm_mode_parse_command_line_for_connector in drm_modes.c
If that doesn't work for you, I think it's better to extend/fix it than add driver module options.
Well, the fb can be set by that option, but the default modeset doesn't seem to honor it. So if you start X modeset driver, it still takes what the driver sets as default. That was the problem I hit.
Oh, I've missed the preferred_bpp hint for the generic modesetting driver. Can we fill that in by using the bpp parsed in the fb helper somehow? -Daniel
Hi Dave,
any chance to take a look at this problem?
thanks,
Takashi
At Fri, 25 Jan 2013 17:21:54 +0100, Takashi Iwai wrote:
When the mode is set with 16bpp on QEMU, the output gets totally broken. The culprit is the bogus register values set for 16bpp, which was likely copied from from a wrong place.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=799216
Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de
drivers/gpu/drm/cirrus/cirrus_mode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index 60685b2..379a47e 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -273,8 +273,8 @@ static int cirrus_crtc_mode_set(struct drm_crtc *crtc, sr07 |= 0x11; break; case 16:
sr07 |= 0xc1;hdr = 0xc0;
sr07 |= 0x17;hdr = 0xc1;-- 1.8.1.1
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Takashi Iwai