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.