* [PATCH 1/2] drm/cirrus: Correct register values for 16bpp @ 2013-01-25 16:21 Takashi Iwai 2013-01-25 16:21 ` [PATCH 2/2] drm/cirrus: Use 16bpp as default Takashi Iwai 2013-01-29 8:26 ` [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai 0 siblings, 2 replies; 7+ messages in thread From: Takashi Iwai @ 2013-01-25 16:21 UTC (permalink / raw) To: David Airlie; +Cc: Stefan Dirsch, Bernhard Wiedemann, dri-devel, linux-kernel 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; -- 1.8.1.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/cirrus: Use 16bpp as default 2013-01-25 16:21 [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai @ 2013-01-25 16:21 ` Takashi Iwai 2013-01-29 8:29 ` [PATCH] drm/cirrus: Add bpp option Takashi Iwai 2013-01-29 8:26 ` [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai 1 sibling, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2013-01-25 16:21 UTC (permalink / raw) To: David Airlie; +Cc: Stefan Dirsch, Bernhard Wiedemann, dri-devel, linux-kernel 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; -- 1.8.1.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] drm/cirrus: Add bpp option 2013-01-25 16:21 ` [PATCH 2/2] drm/cirrus: Use 16bpp as default Takashi Iwai @ 2013-01-29 8:29 ` Takashi Iwai 2013-01-29 9:53 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2013-01-29 8:29 UTC (permalink / raw) To: David Airlie; +Cc: Stefan Dirsch, Bernhard Wiedemann, dri-devel, linux-kernel 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; -- 1.8.1.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/cirrus: Add bpp option 2013-01-29 8:29 ` [PATCH] drm/cirrus: Add bpp option Takashi Iwai @ 2013-01-29 9:53 ` Daniel Vetter 2013-01-29 9:57 ` Takashi Iwai 0 siblings, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2013-01-29 9:53 UTC (permalink / raw) To: Takashi Iwai; +Cc: David Airlie, dri-devel, Bernhard Wiedemann, linux-kernel 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/cirrus: Add bpp option 2013-01-29 9:53 ` Daniel Vetter @ 2013-01-29 9:57 ` Takashi Iwai 2013-01-29 10:03 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2013-01-29 9:57 UTC (permalink / raw) To: Daniel Vetter; +Cc: David Airlie, dri-devel, Bernhard Wiedemann, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/cirrus: Add bpp option 2013-01-29 9:57 ` Takashi Iwai @ 2013-01-29 10:03 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2013-01-29 10:03 UTC (permalink / raw) To: Takashi Iwai; +Cc: David Airlie, dri-devel, Bernhard Wiedemann, linux-kernel 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/cirrus: Correct register values for 16bpp 2013-01-25 16:21 [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai 2013-01-25 16:21 ` [PATCH 2/2] drm/cirrus: Use 16bpp as default Takashi Iwai @ 2013-01-29 8:26 ` Takashi Iwai 1 sibling, 0 replies; 7+ messages in thread From: Takashi Iwai @ 2013-01-29 8:26 UTC (permalink / raw) To: David Airlie; +Cc: Stefan Dirsch, Bernhard Wiedemann, dri-devel, linux-kernel 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; > break; > case 24: > sr07 |= 0x15; > -- > 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/ > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-01-29 10:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-25 16:21 [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai 2013-01-25 16:21 ` [PATCH 2/2] drm/cirrus: Use 16bpp as default Takashi Iwai 2013-01-29 8:29 ` [PATCH] drm/cirrus: Add bpp option Takashi Iwai 2013-01-29 9:53 ` Daniel Vetter 2013-01-29 9:57 ` Takashi Iwai 2013-01-29 10:03 ` Daniel Vetter 2013-01-29 8:26 ` [PATCH 1/2] drm/cirrus: Correct register values for 16bpp Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.