On Thu, Feb 01, 2018 at 02:04:46PM +0100, Linus Walleij 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 typically not for this hardware.
>
> 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 and cap the depth to the maximum
> supported.
>
> 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.
>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 40 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e56166334455..5076f9103740 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1720,6 +1720,8 @@ 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;
> + struct drm_plane *plane;
> + int best_depth = 0;
>
> memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
> sizes.surface_depth = 24;
> @@ -1727,7 +1729,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;
>
> @@ -1762,6 +1767,39 @@ 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.
> + */
> + drm_for_each_plane(plane, fb_helper->dev) {
> + /* Only check the primary plane */
> + if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> + continue;
I think this should look at crtc->primary for each of the crtcs managed
by the fb_helper.
Also this probably shouldn't look at YUV formats at all?
I do wonder if instead we should just have the driver specify the
pixel format explicitly instead of trying to guess based on bpp?
> +
> + for (i = 0; i < plane->format_count; i++) {
> + const struct drm_format_info *fmt;
> +
> + fmt = drm_format_info(plane->format_types[i]);
> + /* 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_DEBUG("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.14.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
