[PATCH v2] drm/fb-helper: Scale back depth to supported maximum

[Daniel Vetter]

On Fri, Dec 28, 2018 at 4:40 PM Linus Walleij <linus.walleij at linaro.org> wrote:
>
> The following happened when migrating an old fbdev driver to DRM:
>
> The Integrator/CP PL111 supports 16BPP but only ARGB1555/ABGR1555
> or XRGB1555/XBGR1555 i.e. the maximum depth is 15.
>
> This makes the initialization of the framebuffer fail since
> the code in drm_fb_helper_single_fb_probe() assigns the same value
> to sizes.surface_bpp and sizes.surface_depth. I.e. it simply assumes
> a 1-to-1 mapping between BPP and depth, which is true in most cases
> but not for this hardware that only support odd formats.
>
> To support the odd case of a driver supporting 16BPP with only 15
> bits of depth, this patch will make the code loop over the formats
> supported on the primary plane on each CRTC managed by the FB
> helper and cap the depth to the maximum supported on any primary
> plane.
>
> On the PL110 Integrator, this makes drm_mode_legacy_fb_format()
> select DRM_FORMAT_XRGB1555 which is acceptable for this driver, and
> thus we get framebuffer, penguin and console on the Integrator/CP.
>
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> ChangeLog v1->v2:
> - Loop over the CRTCs managed by the helper and check the
>   crtc->primary on each CRTC for the applicable formats and
>   thus depths.
> - Skip over YUV formats. The framebuffer emulation cannot
>   handle these formats.
>
> The v1 was sent some while back in february and I only recently
> got back to fixing this up to support the last CLCD displays.
> It was agreed that it is probably best to augment the framebuffer
> initializer to pass a desired pixel format instead of just
> BPP as today, but that is a bit daunting, and Daniel said that
> we would probably anyways need a fallback like this.
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d3af098b0922..57be06d932e4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1797,6 +1797,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>         int i;
>         struct drm_fb_helper_surface_size sizes;
>         int gamma_size = 0;
> +       int best_depth = 0;
>
>         memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
>         sizes.surface_depth = 24;
> @@ -1804,7 +1805,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>         sizes.fb_width = (u32)-1;
>         sizes.fb_height = (u32)-1;
>
> -       /* if driver picks 8 or 16 by default use that for both depth/bpp */
> +       /*
> +        * If driver picks 8 or 16 by default use that for both depth/bpp
> +        * to begin with
> +        */
>         if (preferred_bpp != sizes.surface_bpp)
>                 sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
>
> @@ -1839,6 +1843,47 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>                 }
>         }
>
> +       /*
> +        * If we run into a situation where, for example, the primary plane
> +        * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
> +        * 16) we need to scale down the depth of the sizes we request.
> +        */
> +       for (i = 0; i < fb_helper->crtc_count; i++) {
> +               struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> +               struct drm_crtc *crtc = mode_set->crtc;
> +               struct drm_plane *plane = crtc->primary;
> +               int j;
> +
> +               DRM_DEBUG("test CRTC %d primary plane\n", i);
> +
> +               for (j = 0; j < plane->format_count; j++) {
> +                       const struct drm_format_info *fmt;
> +
> +                       fmt = drm_format_info(plane->format_types[j]);
> +
> +                       /* Do not consider YUV formats for framebuffers */
> +                       if (fmt->is_yuv)

I think better to skip all funny formats with fmt->depth == 0. With that:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> +                               continue;
> +
> +                       /* We found a perfect fit, great */
> +                       if (fmt->depth == sizes.surface_depth)
> +                               break;
> +
> +                       /* Skip depths above what we're looking for */
> +                       if (fmt->depth > sizes.surface_depth)
> +                               continue;
> +
> +                       /* Best depth found so far */
> +                       if (fmt->depth > best_depth)
> +                               best_depth = fmt->depth;
> +               }
> +       }
> +       if (sizes.surface_depth != best_depth) {
> +               DRM_INFO("requested bpp %d, scaled depth down to %d",
> +                         sizes.surface_bpp, best_depth);
> +               sizes.surface_depth = best_depth;
> +       }
> +
>         crtc_count = 0;
>         for (i = 0; i < fb_helper->crtc_count; i++) {
>                 struct drm_display_mode *desired_mode;
> --
> 2.19.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch