Re: [PATCH 3.17] drm/msm: Fix fbdev for 16- and 24-bit modes.

[Daniel Thompson <daniel.thompson@linaro.org>]

On 17/10/14 15:43, Rob Clark wrote:
> On Fri, Oct 17, 2014 at 10:28 AM, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
>> Currently forcing the video mode from the kernel command line (for example
>> video=HDMI-A-1:1280x720-16@60) does not correctly set the number of bits
>> per pixel. This is due to a rather aggressive override in
>> msm_fbdev_create(). This is a particular problem for Android bring up
>> because the software EGL fallbacks don't support 32bpp.
>>
>> This problem could be trivially fixed by removing the override completely.
>> However to avoid regressing commit 96673ecbd7f6 ("drm/msm: default to
>> XR24 rather than AR24") this patch continues to override the depth if
>> there are 32 bits per pixel.
>>
>> Change was tested by dd'ing a test image to /dev/fb0 with no video=
>> (still 32bpp, DBG() message observed), video=1920x1080-32@60 (message
>> observed), video=1920x1080-24@60 and video=1920x1080-16@60 .
>>
>> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/gpu/drm/msm/msm_fbdev.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
>> index ab5bfd2..ed0171d 100644
>> --- a/drivers/gpu/drm/msm/msm_fbdev.c
>> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
>> @@ -93,8 +93,10 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
>>         uint32_t paddr;
>>         int ret, size;
>>
>> -       sizes->surface_bpp = 32;
>> -       sizes->surface_depth = 24;
>> +       if (sizes->surface_bpp == 32) {
>> +               DBG("forcing surface depth to 24\n");
>> +               sizes->surface_depth = 24;
>> +       }
> 
> Thanks.. we could probably even drop this forcing depth to 24.. from a
> quick check in drm_fb_helper_single_fb_probe() the default is 24/32
> which should be enough for the "flicker-free" handover from splash
> screen to X.

Interesting...

I probably should have checked that myself. "git blame" told me that the
change to surface_depth came a long time after forcing the surface_bpp
so I figured it was needed.

v2 may be forthcoming shortly.



> Not really sure why I was forcing the depth/bpp.. I think copy/paste
> of a hack from another driver to do video 'underlays' ;-)
> 
> BR,
> -R
> 
>>
>>         DBG("create fbdev: %dx%d@%d (%dx%d)", sizes->surface_width,
>>                         sizes->surface_height, sizes->surface_bpp,
>> --
>> 1.9.3
>>