On Mon, 2019-01-07 at 11:08 +0100, Daniel Vetter wrote:
> > > > @@ -1654,6 +1712,40 @@ int drm_fb_helper_check_var(struct
> > > > fb_var_screeninfo *var,
> > > > return -EINVAL;
> > > > }
> > > >
> > > > + /*
> > > > + * Workaround for SDL 1.2, which is known to be setting all
> > > > pixel format
> > > > + * fields values to zero in some cases. We treat this situation
> > > > as a
> > > > + * kind of "use some reasonable autodetected values".
> > > > + */
> > > > + if (!var->red.offset && !var->green.offset &&
> > > > + !var->blue.offset && !var->transp.offset &&
> > > > + !var->red.length && !var->green.length &&
> > > > + !var->blue.length && !var->transp.length &&
> > > > + !var->red.msb_right && !var->green.msb_right &&
> > > > + !var->blue.msb_right && !var->transp.msb_right) {
> > > > + u8 depth;
> > > > +
> > > > + /*
> > > > + * There is no way to guess the right value for depth
> > > > when
> > > > + * bpp is 16 or 32. So we just restore the behaviour
> > > > previously
> > > > + * introduced here by commit . In fact, this was
> > > > + * implemented even earlier in various device drivers.
> > > > + */
> > > > + switch (var->bits_per_pixel) {
> > > > + case 16:
> > > > + depth = 15;
> > > > + break;
> > > > + case 32:
> > > > + depth = 24;
> > > > + break;
> > > > + default:
> > > > + depth = var->bits_per_pixel;
> > > > + break;
> > > > + }
> > > > +
> > > > + drm_fb_helper_fill_pixel_fmt(var, depth);
> > >
> > > Please use fb->format->depth here instead of guessing.
> > > -Daniel
> >
> > I do not think that this is the right way.
> >
> > Without guessing, if SDL1 makes a request with bits_per_pixel == 16
> > (for example) and current fb->format->depth == 24, ioctl() will succeed
> > while actual pixel format will remain the same. As a result, SDL1 will
> > display garbage because of invalid pixel format.
> >
> > Or am I missing something here?
>
> This is supposed to be the case where SDL didn't set any of this stuff.
> Guess is definitely not what we want to do, we should use the actual
> depth, which is stored in fb->format->depth. Older code did guess, but we
> fixed that, and shouldn't reintroduce that guess game.
> -Daniel
>
Done. See the v2 patch series.
