On Fri, Dec 28, 2018 at 4:40 PM Linus Walleij <linus.walleij@xxxxxxxxxx> 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@xxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> 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@xxxxxxxx>
> + 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
