On Fri, Dec 28, 2018 at 04:26:56AM +0500, Ivan Mironov wrote:
> On Thu, 2018-12-27 at 13:00 +0100, Daniel Vetter wrote:
> > > + /*
> > > + * 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 785b93ef8c309. 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;
> > > + }
> >
> > The guesswork here looks fishy. We should still have the drm-side format,
> > and should use that.
>
> This existed for a very long time until problematic commit was applied.
> And there is a clear evidence that it was actually used by
> applications.
I'm not against guessing this stuff, but we know have much better format
handling code than when this code was originally written. I just want to
use that (like it's used everywhere else in this file now). fb->format
gives you the right depth, no guessing needed at all.
And if you guess wrong here, you'll fail in these format checks later on.
-Daniel
>
> > Otherwise the patches look good I think, but they
> > need a Fixes: tag and cc: stable so backporters know what to do with
> > these.
> >
>
> I added "cc: stable" into the regression fix. Also added more info into
> the commit messages. See the PATCH v1 in the mailing list.
>
> Thanks.
>
>
>
--
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
