[xserver] modesetting: Honor xorg.conf for 16bpp

Submitted by Adam Jackson on March 21, 2017, 7:57 p.m.

Details

Message ID 20170321195730.1750-1-ajax@redhat.com
State New
Series "modesetting: Honor xorg.conf for 16bpp"
Headers show

Commit Message

Adam Jackson March 21, 2017, 7:57 p.m.
The 32->24 support patch messed this up.

Bugzilla: https://bugs.freedesktop.org/100246
Bugzilla: https://bugs.freedesktop.org/100295
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 hw/xfree86/drivers/modesetting/driver.c          | 4 ++++
 hw/xfree86/drivers/modesetting/drmmode_display.c | 6 ++++++
 2 files changed, 10 insertions(+)

Patch hide | download patch | download mbox

diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
index d7030e5..762b398 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -922,6 +922,8 @@  PreInit(ScrnInfoPtr pScrn, int flags)
     if (!check_outputs(ms->fd, &connector_count))
         return FALSE;
 
+    defaultdepth = pScrn->confScreen->defaultdepth;
+    defaultbpp = pScrn->confScreen->defaultbpp;
     drmmode_get_default_bpp(pScrn, &ms->drmmode, &defaultdepth, &defaultbpp);
     if (defaultdepth == 24 && defaultbpp == 24) {
         ms->drmmode.force_24_32 = TRUE;
@@ -949,6 +951,8 @@  PreInit(ScrnInfoPtr pScrn, int flags)
                    pScrn->depth);
         return FALSE;
     }
+    if (ms->drmmode.kbpp == 0)
+        ms->drmmode.kbpp = pScrn->bitsPerPixel;
     xf86PrintDepthBpp(pScrn);
 
     /* Process the options */
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index c1e489e..e6158ab 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -2057,6 +2057,8 @@  drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
     xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
 
     drmmode->scrn = pScrn;
+    if (drmmode->force_24_32 && cpp == 4)
+        cpp = 3;
     drmmode->cpp = cpp;
     mode_res = drmModeGetResources(drmmode->fd);
     if (!mode_res)
@@ -2488,6 +2490,10 @@  drmmode_get_default_bpp(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int *depth,
     uint32_t fb_id;
     int ret;
 
+    /* if we've been configured in xorg.conf, trust it */
+    if (*depth || *bpp)
+        return;
+
     /* 16 is fine */
     ret = drmGetCap(drmmode->fd, DRM_CAP_DUMB_PREFERRED_DEPTH, &value);
     if (!ret && (value == 16 || value == 8)) {

Comments

Hans de Goede March 22, 2017, 2:49 p.m.
Hi,

On 21-03-17 20:57, Adam Jackson wrote:
> The 32->24 support patch messed this up.
>
> Bugzilla: https://bugs.freedesktop.org/100246
> Bugzilla: https://bugs.freedesktop.org/100295
> Signed-off-by: Adam Jackson <ajax@redhat.com>

LGTM:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  hw/xfree86/drivers/modesetting/driver.c          | 4 ++++
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 6 ++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
> index d7030e5..762b398 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -922,6 +922,8 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>      if (!check_outputs(ms->fd, &connector_count))
>          return FALSE;
>
> +    defaultdepth = pScrn->confScreen->defaultdepth;
> +    defaultbpp = pScrn->confScreen->defaultbpp;
>      drmmode_get_default_bpp(pScrn, &ms->drmmode, &defaultdepth, &defaultbpp);
>      if (defaultdepth == 24 && defaultbpp == 24) {
>          ms->drmmode.force_24_32 = TRUE;
> @@ -949,6 +951,8 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>                     pScrn->depth);
>          return FALSE;
>      }
> +    if (ms->drmmode.kbpp == 0)
> +        ms->drmmode.kbpp = pScrn->bitsPerPixel;
>      xf86PrintDepthBpp(pScrn);
>
>      /* Process the options */
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index c1e489e..e6158ab 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -2057,6 +2057,8 @@ drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
>      xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
>
>      drmmode->scrn = pScrn;
> +    if (drmmode->force_24_32 && cpp == 4)
> +        cpp = 3;
>      drmmode->cpp = cpp;
>      mode_res = drmModeGetResources(drmmode->fd);
>      if (!mode_res)
> @@ -2488,6 +2490,10 @@ drmmode_get_default_bpp(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int *depth,
>      uint32_t fb_id;
>      int ret;
>
> +    /* if we've been configured in xorg.conf, trust it */
> +    if (*depth || *bpp)
> +        return;
> +
>      /* 16 is fine */
>      ret = drmGetCap(drmmode->fd, DRM_CAP_DUMB_PREFERRED_DEPTH, &value);
>      if (!ret && (value == 16 || value == 8)) {
>
Timo Aaltonen March 23, 2017, 9 a.m.
On 21.03.2017 21:57, Adam Jackson wrote:
> The 32->24 support patch messed this up.
> 
> Bugzilla: https://bugs.freedesktop.org/100246
> Bugzilla: https://bugs.freedesktop.org/100295
> Signed-off-by: Adam Jackson <ajax@redhat.com>

'xinit -- -depth 16' still doesn't work, but xorg.conf does.
Alkis Georgopoulos March 23, 2017, 9:20 a.m.
On 23/03/2017 11:00 πμ, Timo Aaltonen wrote:
> 'xinit -- -depth 16' still doesn't work, but xorg.conf does.
>

I verify this on kvm/cirrus/modeset and on vbox/vboxvideo/modeset.
It also fixes the crash on Intel broadwell with modesetting!

But without a xorg.conf, i.e. with `xinit -- -depth 16`, everything is 
still broken.
Adam Jackson March 23, 2017, 6:04 p.m.
On Thu, 2017-03-23 at 11:20 +0200, Alkis Georgopoulos wrote:
> On 23/03/2017 11:00 πμ, Timo Aaltonen wrote:
> > 'xinit -- -depth 16' still doesn't work, but xorg.conf does.
> > 
> 
> I verify this on kvm/cirrus/modeset and on vbox/vboxvideo/modeset.
> It also fixes the crash on Intel broadwell with modesetting!
> 
> But without a xorg.conf, i.e. with `xinit -- -depth 16`, everything is 
> still broken.

Ugh, yeah. The issue there is the command line options aren't directly
visible to the drivers. I'll see if I can work something out.

- ajax