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

[Noralf Trønnes]

Den 01.02.2018 14.19, skrev Ville Syrjälä:
> 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 at linaro.org>
>> ---
>>   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?

Can drm_mode_config.preferred_depth be used for this? The comment says that
it is used for dumb buffers and drm_mode_create_dumb_ioctl() does this:
     cpp = DIV_ROUND_UP(args->bpp, 8);

So it looks like it should be possible to do preferred_depth=15.

Noralf.

>> +
>> +		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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel