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

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(-)

--
1.9.3

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.

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
>

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
>>