| Message ID | 20201009232156.3916879-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/vkms: Set preferred depth correctly | expand |
On Sat, 10 Oct 2020 01:21:54 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > The only thing we support is xrgb8888. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > Cc: Melissa Wen <melissa.srw@gmail.com> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 726801ab44d4..eb4007443706 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > dev->mode_config.max_height = YRES_MAX; > dev->mode_config.cursor_width = 512; > dev->mode_config.cursor_height = 512; > - dev->mode_config.preferred_depth = 24; > + dev->mode_config.preferred_depth = 32; > dev->mode_config.helper_private = &vkms_mode_config_helpers; > > return vkms_output_init(vkmsdev, 0);
On 10/10, Daniel Vetter wrote: > The only thing we support is xrgb8888. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > Cc: Melissa Wen <melissa.srw@gmail.com> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 726801ab44d4..eb4007443706 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > dev->mode_config.max_height = YRES_MAX; > dev->mode_config.cursor_width = 512; > dev->mode_config.cursor_height = 512; > - dev->mode_config.preferred_depth = 24; > + dev->mode_config.preferred_depth = 32; nice catch! > dev->mode_config.helper_private = &vkms_mode_config_helpers; > > return vkms_output_init(vkmsdev, 0); > -- > 2.28.0 > Thanks, Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
On Mon, Oct 12, 2020 at 09:59:22AM -0300, Melissa Wen wrote: > On 10/10, Daniel Vetter wrote: > > The only thing we support is xrgb8888. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > Cc: Melissa Wen <melissa.srw@gmail.com> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index 726801ab44d4..eb4007443706 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > dev->mode_config.max_height = YRES_MAX; > > dev->mode_config.cursor_width = 512; > > dev->mode_config.cursor_height = 512; > > - dev->mode_config.preferred_depth = 24; > > + dev->mode_config.preferred_depth = 32; > nice catch! > > > dev->mode_config.helper_private = &vkms_mode_config_helpers; > > > > return vkms_output_init(vkmsdev, 0); > > -- > > 2.28.0 > > > Thanks, > > Reviewed-by: Melissa Wen <melissa.srw@gmail.com> I merged the first 2 patches of this series, but noticed that you didn't reply with a r-b tag on the 3rd patch. Is that intentional or just oversight? Thanks, Daniel
On 10/15, Daniel Vetter wrote: > On Mon, Oct 12, 2020 at 09:59:22AM -0300, Melissa Wen wrote: > > On 10/10, Daniel Vetter wrote: > > > The only thing we support is xrgb8888. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > Cc: Melissa Wen <melissa.srw@gmail.com> > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > --- > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > index 726801ab44d4..eb4007443706 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > dev->mode_config.max_height = YRES_MAX; > > > dev->mode_config.cursor_width = 512; > > > dev->mode_config.cursor_height = 512; > > > - dev->mode_config.preferred_depth = 24; > > > + dev->mode_config.preferred_depth = 32; > > nice catch! > > > > > dev->mode_config.helper_private = &vkms_mode_config_helpers; > > > > > > return vkms_output_init(vkmsdev, 0); > > > -- > > > 2.28.0 > > > > > Thanks, > > > > Reviewed-by: Melissa Wen <melissa.srw@gmail.com> > > I merged the first 2 patches of this series, but noticed that you didn't > reply with a r-b tag on the 3rd patch. Is that intentional or just > oversight? Hi Daniel, Initially, it was intentional because I was following the feedback and still wanted to check it out better, then I forgot to come back to comment. My fault, but it's done now. Thanks for the touch, Melissa > > Thanks, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
> The only thing we support is xrgb8888. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > Cc: Melissa Wen <melissa.srw@gmail.com> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 726801ab44d4..eb4007443706 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > dev->mode_config.max_height = YRES_MAX; > dev->mode_config.cursor_width = 512; > dev->mode_config.cursor_height = 512; > - dev->mode_config.preferred_depth = 24; > + dev->mode_config.preferred_depth = 32; Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* drivers set it to 24. [1]: https://drmdb.emersion.fr/capabilities > dev->mode_config.helper_private = &vkms_mode_config_helpers; > > return vkms_output_init(vkmsdev, 0); > -- > 2.28.0
On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > > > The only thing we support is xrgb8888. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > Cc: Melissa Wen <melissa.srw@gmail.com> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index 726801ab44d4..eb4007443706 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > dev->mode_config.max_height = YRES_MAX; > > dev->mode_config.cursor_width = 512; > > dev->mode_config.cursor_height = 512; > > - dev->mode_config.preferred_depth = 24; > > + dev->mode_config.preferred_depth = 32; > > Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > drivers set it to 24. Uh there's a problem I think. Depth should indeed stay at 24, the problem is that fb helpers directly take that to be the bpp parameter, which is a different thing. And if you look at how most drivers set up that, they pick 32. I guess I need to revert this here, and unconfuse the fb helpers about depth vs bpp. Maybe best would be to just switch over to preferred drm_fourcc format code, or maybe just pick this up from the first format the primary plane supports. This is all getting slightly tricky and a lot more work :-/ -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On 10/16, Daniel Vetter wrote: > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > > > > > The only thing we support is xrgb8888. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > Cc: Melissa Wen <melissa.srw@gmail.com> > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > --- > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > index 726801ab44d4..eb4007443706 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > dev->mode_config.max_height = YRES_MAX; > > > dev->mode_config.cursor_width = 512; > > > dev->mode_config.cursor_height = 512; > > > - dev->mode_config.preferred_depth = 24; > > > + dev->mode_config.preferred_depth = 32; > > > > Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > > drivers set it to 24. > > Uh there's a problem I think. Depth should indeed stay at 24, the > problem is that fb helpers directly take that to be the bpp parameter, > which is a different thing. And if you look at how most drivers set up > that, they pick 32. > > I guess I need to revert this here, and unconfuse the fb helpers about > depth vs bpp. Hi guys, Perhaps it deserves to be pointed out: the documentation says "preferred_depth: preferred RBG(sic) pixel depth, used by fb helpers", and looking to fb helpers, preferred_depth is only used by generic_setup, as bits by pixel (if I didn't miss something there). In fact, the alpha channel is not used for final display (perhaps in the future); however, I saw another driver with a change similar to this here and, possibly like me, following the same misunderstanding. Melissa > > Maybe best would be to just switch over to preferred drm_fourcc format > code, or maybe just pick this up from the first format the primary > plane supports. > > This is all getting slightly tricky and a lot more work :-/ > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Fri, Oct 16, 2020 at 7:02 PM Melissa Wen <melissa.srw@gmail.com> wrote: > > On 10/16, Daniel Vetter wrote: > > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > > > > > > > The only thing we support is xrgb8888. > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > Cc: Melissa Wen <melissa.srw@gmail.com> > > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > --- > > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > > index 726801ab44d4..eb4007443706 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > > dev->mode_config.max_height = YRES_MAX; > > > > dev->mode_config.cursor_width = 512; > > > > dev->mode_config.cursor_height = 512; > > > > - dev->mode_config.preferred_depth = 24; > > > > + dev->mode_config.preferred_depth = 32; > > > > > > Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > > > drivers set it to 24. > > > > Uh there's a problem I think. Depth should indeed stay at 24, the > > problem is that fb helpers directly take that to be the bpp parameter, > > which is a different thing. And if you look at how most drivers set up > > that, they pick 32. > > > > I guess I need to revert this here, and unconfuse the fb helpers about > > depth vs bpp. > > Hi guys, > > Perhaps it deserves to be pointed out: the documentation says > "preferred_depth: preferred RBG(sic) pixel depth, used by fb helpers", > and looking to fb helpers, preferred_depth is only used by > generic_setup, as bits by pixel (if I didn't miss something there). > > In fact, the alpha channel is not used for final display (perhaps in the > future); however, I saw another driver with a change similar to this > here and, possibly like me, following the same misunderstanding. Yeah the problem is that preferred_depth is depth, and that means 24 bit for XRGB8888. But bpp as used by fb helpers would be 32 bit for XRGB8888. I think the real fix here is to switch this entire mess over to using drm_fourcc codes directly, at least for atomic drivers. Which nowadays are most. Interim I'm not sure whether we should revert my patch (it breaks fbdev) or switch preferred_depth to 0, which means we get the default every, and that means both fbdev helpers and userspace will pick XRGB8888. -Daniel > Melissa > > > > Maybe best would be to just switch over to preferred drm_fourcc format > > code, or maybe just pick this up from the first format the primary > > plane supports. > > > > This is all getting slightly tricky and a lot more work :-/ > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
On 10/16, Daniel Vetter wrote: > On Fri, Oct 16, 2020 at 7:02 PM Melissa Wen <melissa.srw@gmail.com> wrote: > > > > On 10/16, Daniel Vetter wrote: > > > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > > > > > > > > > The only thing we support is xrgb8888. > > > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > > Cc: Melissa Wen <melissa.srw@gmail.com> > > > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > > --- > > > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > > > index 726801ab44d4..eb4007443706 100644 > > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > > > dev->mode_config.max_height = YRES_MAX; > > > > > dev->mode_config.cursor_width = 512; > > > > > dev->mode_config.cursor_height = 512; > > > > > - dev->mode_config.preferred_depth = 24; > > > > > + dev->mode_config.preferred_depth = 32; > > > > > > > > Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > > > > drivers set it to 24. > > > > > > Uh there's a problem I think. Depth should indeed stay at 24, the > > > problem is that fb helpers directly take that to be the bpp parameter, > > > which is a different thing. And if you look at how most drivers set up > > > that, they pick 32. > > > > > > I guess I need to revert this here, and unconfuse the fb helpers about > > > depth vs bpp. > > > > Hi guys, > > > > Perhaps it deserves to be pointed out: the documentation says > > "preferred_depth: preferred RBG(sic) pixel depth, used by fb helpers", > > and looking to fb helpers, preferred_depth is only used by > > generic_setup, as bits by pixel (if I didn't miss something there). > > > > In fact, the alpha channel is not used for final display (perhaps in the > > future); however, I saw another driver with a change similar to this > > here and, possibly like me, following the same misunderstanding. > > Yeah the problem is that preferred_depth is depth, and that means 24 > bit for XRGB8888. But bpp as used by fb helpers would be 32 bit for > XRGB8888. > > I think the real fix here is to switch this entire mess over to using > drm_fourcc codes directly, at least for atomic drivers. Which nowadays > are most. Interim I'm not sure whether we should revert my patch (it > breaks fbdev) or switch preferred_depth to 0, which means we get the > default every, and that means both fbdev helpers and userspace will > pick XRGB8888. hmm... but why not keep preferred_depth = 24 and pass 32 as the preferred_bpp parameter of fbdev_generic_setup? > -Daniel > > > Melissa > > > > > > Maybe best would be to just switch over to preferred drm_fourcc format > > > code, or maybe just pick this up from the first format the primary > > > plane supports. > > > > > > This is all getting slightly tricky and a lot more work :-/ > > > -Daniel > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Sat, Oct 17, 2020 at 10:39 AM Melissa Wen <melissa.srw@gmail.com> wrote: > > On 10/16, Daniel Vetter wrote: > > On Fri, Oct 16, 2020 at 7:02 PM Melissa Wen <melissa.srw@gmail.com> wrote: > > > > > > On 10/16, Daniel Vetter wrote: > > > > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > > > > > > > > > > > The only thing we support is xrgb8888. > > > > > > > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > > > Cc: Melissa Wen <melissa.srw@gmail.com> > > > > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > > > --- > > > > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > > > > index 726801ab44d4..eb4007443706 100644 > > > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > > > > @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > > > > dev->mode_config.max_height = YRES_MAX; > > > > > > dev->mode_config.cursor_width = 512; > > > > > > dev->mode_config.cursor_height = 512; > > > > > > - dev->mode_config.preferred_depth = 24; > > > > > > + dev->mode_config.preferred_depth = 32; > > > > > > > > > > Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > > > > > drivers set it to 24. > > > > > > > > Uh there's a problem I think. Depth should indeed stay at 24, the > > > > problem is that fb helpers directly take that to be the bpp parameter, > > > > which is a different thing. And if you look at how most drivers set up > > > > that, they pick 32. > > > > > > > > I guess I need to revert this here, and unconfuse the fb helpers about > > > > depth vs bpp. > > > > > > Hi guys, > > > > > > Perhaps it deserves to be pointed out: the documentation says > > > "preferred_depth: preferred RBG(sic) pixel depth, used by fb helpers", > > > and looking to fb helpers, preferred_depth is only used by > > > generic_setup, as bits by pixel (if I didn't miss something there). > > > > > > In fact, the alpha channel is not used for final display (perhaps in the > > > future); however, I saw another driver with a change similar to this > > > here and, possibly like me, following the same misunderstanding. > > > > Yeah the problem is that preferred_depth is depth, and that means 24 > > bit for XRGB8888. But bpp as used by fb helpers would be 32 bit for > > XRGB8888. > > > > I think the real fix here is to switch this entire mess over to using > > drm_fourcc codes directly, at least for atomic drivers. Which nowadays > > are most. Interim I'm not sure whether we should revert my patch (it > > breaks fbdev) or switch preferred_depth to 0, which means we get the > > default every, and that means both fbdev helpers and userspace will > > pick XRGB8888. > > hmm... but why not keep preferred_depth = 24 and pass 32 as the > preferred_bpp parameter of fbdev_generic_setup? The goal is to get rid of this parameter in fbdev_generic_setup. It should be able to figure this out automatically, like any kms client in userspace. What does work is setting preferred_bpp = 0. Userspace will pick the default (which is generally 24 depth, 32bpp), and fbcon do the same. And then I guess a nice patch series to clean up this mess. -Daniel > > -Daniel > > > > > Melissa > > > > > > > > Maybe best would be to just switch over to preferred drm_fourcc format > > > > code, or maybe just pick this up from the first format the primary > > > > plane supports. > > > > > > > > This is all getting slightly tricky and a lot more work :-/ > > > > -Daniel > > > > -- > > > > Daniel Vetter > > > > Software Engineer, Intel Corporation > > > > http://blog.ffwll.ch > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
Hi Am 16.10.20 um 13:35 schrieb Daniel Vetter: > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: >> >>> The only thing we support is xrgb8888. >>> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> >>> Cc: Melissa Wen <melissa.srw@gmail.com> >>> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> >>> Cc: Daniel Vetter <daniel@ffwll.ch> >>> --- >>> drivers/gpu/drm/vkms/vkms_drv.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c >>> index 726801ab44d4..eb4007443706 100644 >>> --- a/drivers/gpu/drm/vkms/vkms_drv.c >>> +++ b/drivers/gpu/drm/vkms/vkms_drv.c >>> @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) >>> dev->mode_config.max_height = YRES_MAX; >>> dev->mode_config.cursor_width = 512; >>> dev->mode_config.cursor_height = 512; >>> - dev->mode_config.preferred_depth = 24; >>> + dev->mode_config.preferred_depth = 32; >> >> Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* >> drivers set it to 24. > > Uh there's a problem I think. Depth should indeed stay at 24, the > problem is that fb helpers directly take that to be the bpp parameter, > which is a different thing. And if you look at how most drivers set up > that, they pick 32. > > I guess I need to revert this here, and unconfuse the fb helpers about > depth vs bpp. I just prepared the PR for drm-misc-next, but saw it at 32 still. Was this supposed to be reverted? Best regards Thomas > > Maybe best would be to just switch over to preferred drm_fourcc format > code, or maybe just pick this up from the first format the primary > plane supports. > > This is all getting slightly tricky and a lot more work :-/ > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Tue, Oct 27, 2020 at 11:13 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > Am 16.10.20 um 13:35 schrieb Daniel Vetter: > > On Fri, Oct 16, 2020 at 12:38 PM Simon Ser <contact@emersion.fr> wrote: > >> > >>> The only thing we support is xrgb8888. > >>> > >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >>> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > >>> Cc: Melissa Wen <melissa.srw@gmail.com> > >>> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > >>> Cc: Daniel Vetter <daniel@ffwll.ch> > >>> --- > >>> drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > >>> index 726801ab44d4..eb4007443706 100644 > >>> --- a/drivers/gpu/drm/vkms/vkms_drv.c > >>> +++ b/drivers/gpu/drm/vkms/vkms_drv.c > >>> @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > >>> dev->mode_config.max_height = YRES_MAX; > >>> dev->mode_config.cursor_width = 512; > >>> dev->mode_config.cursor_height = 512; > >>> - dev->mode_config.preferred_depth = 24; > >>> + dev->mode_config.preferred_depth = 32; > >> > >> Are you sure xrgb8888's depth is 32 and not 24? Looking at drmdb [1], *all* > >> drivers set it to 24. > > > > Uh there's a problem I think. Depth should indeed stay at 24, the > > problem is that fb helpers directly take that to be the bpp parameter, > > which is a different thing. And if you look at how most drivers set up > > that, they pick 32. > > > > I guess I need to revert this here, and unconfuse the fb helpers about > > depth vs bpp. > > I just prepared the PR for drm-misc-next, but saw it at 32 still. Was > this supposed to be reverted? Revert makes fbdev go boom, only thing that would work is leaving it at 0 so that everyone picks the default. I've tried to come up with a patch series to clean up this mess, but it's rather horrible :-/ -Daniel > > Best regards > Thomas > > > > > Maybe best would be to just switch over to preferred drm_fourcc format > > code, or maybe just pick this up from the first format the primary > > plane supports. > > > > This is all getting slightly tricky and a lot more work :-/ > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 726801ab44d4..eb4007443706 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -124,7 +124,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) dev->mode_config.max_height = YRES_MAX; dev->mode_config.cursor_width = 512; dev->mode_config.cursor_height = 512; - dev->mode_config.preferred_depth = 24; + dev->mode_config.preferred_depth = 32; dev->mode_config.helper_private = &vkms_mode_config_helpers; return vkms_output_init(vkmsdev, 0);
The only thing we support is xrgb8888. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Melissa Wen <melissa.srw@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/vkms/vkms_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)