diff mbox series

[4/4] drm/format-helper: Add drm_fb_build_fourcc_list() helper

Message ID 20220810112053.19547-5-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/probe-helper, modes: Helpers for single-mode displays | expand

Commit Message

Thomas Zimmermann Aug. 10, 2022, 11:20 a.m. UTC
Add drm_fb_build_fourcc_list() function that builds a list of supported
formats from native and emulated ones. Helpful for all drivers that do
format conversion as part of their plane updates. Update current caller.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_format_helper.c | 94 +++++++++++++++++++++++++++++
 drivers/gpu/drm/tiny/simpledrm.c    | 47 ++-------------
 include/drm/drm_format_helper.h     | 11 +++-
 3 files changed, 109 insertions(+), 43 deletions(-)

Comments

Sam Ravnborg Aug. 12, 2022, 6:19 p.m. UTC | #1
Hi Thomas,

On Wed, Aug 10, 2022 at 01:20:53PM +0200, Thomas Zimmermann wrote:
> Add drm_fb_build_fourcc_list() function that builds a list of supported
> formats from native and emulated ones. Helpful for all drivers that do
> format conversion as part of their plane updates. Update current caller.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

A few comments in the following. Consider to add the warning and with it
added or not:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/drm_format_helper.c | 94 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c    | 47 ++-------------
>  include/drm/drm_format_helper.h     | 11 +++-
>  3 files changed, 109 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 56642816fdff..dca5531615f3 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -793,3 +793,97 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>  	kfree(src32);
>  }
>  EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
> +
> +static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t fourcc)
> +{
> +	const uint32_t *fourccs_end = fourccs + nfourccs;
> +
> +	while (fourccs < fourccs_end) {
> +		if (*fourccs == fourcc)
> +			return true;
> +		++fourccs;
> +	}
> +	return false;
> +}
> +
> +/**
> + * drm_fb_build_fourcc_list - Filters a list of supported color formats against
> + *                            the device's native formats
> + * @dev: DRM device
> + * @native_fourccs: 4CC codes of natively supported color formats
> + * @native_nfourccs: The number of entries in @native_fourccs
> + * @extra_fourccs: 4CC codes of additionally supported color formats
> + * @extra_nfourccs: The number of entries in @extra_fourccs
> + * @fourccs_out: Returns 4CC codes of supported color formats
> + * @nfourccs_out: The number of available entries in @fourccs_out
> + *
> + * This function create a list of supported color format from natively
> + * supported formats and the emulated formats.  *
Stray '*' at the end of the line.

> + * At a minimum, most userspace programs expect at least support for
> + * XRGB8888 on the primary plane. Devices that have to emulate the
> + * format, and possibly others, can use drm_fb_build_fourcc_list() to
> + * create a list of supported color formats. The returned list can
> + * be handed over to drm_universal_plane_init() et al. Native formats
> + * will go before emulated formats. Other heuristics might be applied
> + * to optimize the order. Formats near the beginning of the list are
> + * usually preferred over formats near the end of the list.
> + *
> + * Returns:
> + * The number of color-formats 4CC codes returned in @fourccs_out.
> + */
> +size_t drm_fb_build_fourcc_list(struct drm_device *dev,
> +				const uint32_t *native_fourccs, size_t native_nfourccs,
> +				const uint32_t *extra_fourccs, size_t extra_nfourccs,
> +				uint32_t *fourccs_out, size_t nfourccs_out)

drm_fourcc.c uses the type u32 for fourcc codes, why no go with the same
here?

I wish we had a better way to express that we have a list of fourcc
codes, for example using list_head. But it is a bad mismatch with
the current drm_universal_plane_init() implementation.

The format negotiation operation in the bridges could benefit too.

> +{
> +	uint32_t *fourccs = fourccs_out;
> +	const uint32_t *fourccs_end = fourccs_out + nfourccs_out;
> +	bool found_native = false;
> +	size_t nfourccs, i;
> +
> +	/* native formats go first */
> +
Drop extra line, capital start
> +	nfourccs = min_t(size_t, native_nfourccs, nfourccs_out);
> +
> +	for (i = 0; i < nfourccs; ++i) {
> +		uint32_t fourcc = native_fourccs[i];
> +
> +		drm_dbg_kms(dev, "adding native format %p4cc\n", &fourcc);
> +
> +		if (!found_native)
> +			found_native = is_listed_fourcc(extra_fourccs, extra_nfourccs, fourcc);
> +		*fourccs = fourcc;
> +		++fourccs;
> +	}
> +
> +	/*
> +	 * The plane's atomic_update helper converts the framebuffer's color format
> +	 * to the native format when copying them to device memory.
> +	 *
> +	 * If there is not a single format supported by both, device and
> +	 * plane, the native formats are likely not supported by the conversion
> +	 * helpers. Therefore *only* support the native formats and add a
> +	 * conversion helper ASAP.
> +	 */
Despite the nice comment I had to think twice. In the end I agree with
this.

> +	if (!found_native) {
> +		drm_warn(dev, "format conversion helpers required to add extra formats\n");
> +		goto out;
> +	}
> +
> +	/* extra formats go second */
> +
Drop extra line, capital start.
> +	nfourccs = min_t(size_t, extra_nfourccs, fourccs_end - fourccs);
> +
> +	for (i = 0; i < nfourccs; ++i) {
> +		uint32_t fourcc = extra_fourccs[i];
> +
> +		if (is_listed_fourcc(native_fourccs, native_nfourccs, fourcc))
> +			continue; /* native formats already went first */
> +		*fourccs = fourcc;
> +		++fourccs;
> +	}
> +
> +out:
> +	return fourccs - fourccs_out;
> +}
It would be prudent to warn if the supplied fourccs_out array is too
small to the provided input formats. simpledrm is about to hit the limit.

> +EXPORT_SYMBOL(drm_fb_build_fourcc_list);
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index cc509154b296..79c9fd6bedf0 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -644,45 +644,6 @@ static struct drm_display_mode simpledrm_mode(unsigned int width,
>  	return mode;
>  }
>  
> -static const uint32_t *simpledrm_device_formats(struct simpledrm_device *sdev,
> -						size_t *nformats_out)
> -{
> -	struct drm_device *dev = &sdev->dev;
> -	size_t i;
> -
> -	if (sdev->nformats)
> -		goto out; /* don't rebuild list on recurring calls */
> -
> -	/* native format goes first */
> -	sdev->formats[0] = sdev->format->format;
> -	sdev->nformats = 1;
> -
> -	/* default formats go second */
> -	for (i = 0; i < ARRAY_SIZE(simpledrm_primary_plane_formats); ++i) {
> -		if (simpledrm_primary_plane_formats[i] == sdev->format->format)
> -			continue; /* native format already went first */
> -		sdev->formats[sdev->nformats] = simpledrm_primary_plane_formats[i];
> -		sdev->nformats++;
> -	}
> -
> -	/*
> -	 * TODO: The simpledrm driver converts framebuffers to the native
> -	 * format when copying them to device memory. If there are more
> -	 * formats listed than supported by the driver, the native format
> -	 * is not supported by the conversion helpers. Therefore *only*
> -	 * support the native format and add a conversion helper ASAP.
> -	 */
> -	if (drm_WARN_ONCE(dev, i != sdev->nformats,
> -			  "format conversion helpers required for %p4cc",
> -			  &sdev->format->format)) {
> -		sdev->nformats = 1;
> -	}
> -
> -out:
> -	*nformats_out = sdev->nformats;
> -	return sdev->formats;
> -}
> -
>  static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  							struct platform_device *pdev)
>  {
> @@ -699,7 +660,6 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  	struct drm_encoder *encoder;
>  	struct drm_connector *connector;
>  	unsigned long max_width, max_height;
> -	const uint32_t *formats;
>  	size_t nformats;
>  	int ret;
>  
> @@ -811,11 +771,14 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  
>  	/* Primary plane */
>  
> -	formats = simpledrm_device_formats(sdev, &nformats);
> +	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
> +					    simpledrm_primary_plane_formats,
> +					    ARRAY_SIZE(simpledrm_primary_plane_formats),
> +					    sdev->formats, ARRAY_SIZE(sdev->formats));
simpledrm_primary_plane_formats is 6 long, with a todo to add 2 more.
So the current array of 8 in sdev->formats is big enough for now.


>  
>  	primary_plane = &sdev->primary_plane;
>  	ret = drm_universal_plane_init(dev, primary_plane, 0, &simpledrm_primary_plane_funcs,
> -				       formats, nformats,
> +				       sdev->formats, nformats,
>  				       simpledrm_primary_plane_format_modifiers,
>  				       DRM_PLANE_TYPE_PRIMARY, NULL);
>  	if (ret)
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index caa181194335..33278870e0d8 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -6,11 +6,15 @@
>  #ifndef __LINUX_DRM_FORMAT_HELPER_H
>  #define __LINUX_DRM_FORMAT_HELPER_H
>  
> -struct iosys_map;
> +#include <linux/types.h>
> +
> +struct drm_device;
>  struct drm_format_info;
>  struct drm_framebuffer;
>  struct drm_rect;
>  
> +struct iosys_map;
> +
>  unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>  				const struct drm_rect *clip);
>  
> @@ -44,4 +48,9 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>  			     const struct iosys_map *src, const struct drm_framebuffer *fb,
>  			     const struct drm_rect *clip);
>  
> +size_t drm_fb_build_fourcc_list(struct drm_device *dev,
> +				const uint32_t *native_fourccs, size_t native_nfourccs,
> +				const uint32_t *extra_fourccs, size_t extra_nfourccs,
> +				uint32_t *fourccs_out, size_t nfourccs_out);
> +
>  #endif /* __LINUX_DRM_FORMAT_HELPER_H */
> -- 
> 2.37.1
Thomas Zimmermann Aug. 16, 2022, 1:38 p.m. UTC | #2
Hi

Am 12.08.22 um 20:19 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Wed, Aug 10, 2022 at 01:20:53PM +0200, Thomas Zimmermann wrote:
>> Add drm_fb_build_fourcc_list() function that builds a list of supported
>> formats from native and emulated ones. Helpful for all drivers that do
>> format conversion as part of their plane updates. Update current caller.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> A few comments in the following. Consider to add the warning and with it
> added or not:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
>> ---
>>   drivers/gpu/drm/drm_format_helper.c | 94 +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/tiny/simpledrm.c    | 47 ++-------------
>>   include/drm/drm_format_helper.h     | 11 +++-
>>   3 files changed, 109 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 56642816fdff..dca5531615f3 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
>> @@ -793,3 +793,97 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>>   	kfree(src32);
>>   }
>>   EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
>> +
>> +static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t fourcc)
>> +{
>> +	const uint32_t *fourccs_end = fourccs + nfourccs;
>> +
>> +	while (fourccs < fourccs_end) {
>> +		if (*fourccs == fourcc)
>> +			return true;
>> +		++fourccs;
>> +	}
>> +	return false;
>> +}
>> +
>> +/**
>> + * drm_fb_build_fourcc_list - Filters a list of supported color formats against
>> + *                            the device's native formats
>> + * @dev: DRM device
>> + * @native_fourccs: 4CC codes of natively supported color formats
>> + * @native_nfourccs: The number of entries in @native_fourccs
>> + * @extra_fourccs: 4CC codes of additionally supported color formats
>> + * @extra_nfourccs: The number of entries in @extra_fourccs
>> + * @fourccs_out: Returns 4CC codes of supported color formats
>> + * @nfourccs_out: The number of available entries in @fourccs_out
>> + *
>> + * This function create a list of supported color format from natively
>> + * supported formats and the emulated formats.  *
> Stray '*' at the end of the line.
> 
>> + * At a minimum, most userspace programs expect at least support for
>> + * XRGB8888 on the primary plane. Devices that have to emulate the
>> + * format, and possibly others, can use drm_fb_build_fourcc_list() to
>> + * create a list of supported color formats. The returned list can
>> + * be handed over to drm_universal_plane_init() et al. Native formats
>> + * will go before emulated formats. Other heuristics might be applied
>> + * to optimize the order. Formats near the beginning of the list are
>> + * usually preferred over formats near the end of the list.
>> + *
>> + * Returns:
>> + * The number of color-formats 4CC codes returned in @fourccs_out.
>> + */
>> +size_t drm_fb_build_fourcc_list(struct drm_device *dev,
>> +				const uint32_t *native_fourccs, size_t native_nfourccs,
>> +				const uint32_t *extra_fourccs, size_t extra_nfourccs,
>> +				uint32_t *fourccs_out, size_t nfourccs_out)
> 
> drm_fourcc.c uses the type u32 for fourcc codes, why no go with the same
> here?
> 
> I wish we had a better way to express that we have a list of fourcc
> codes, for example using list_head. But it is a bad mismatch with
> the current drm_universal_plane_init() implementation.

I've changed the code to u32. struct list_head is somewhat overhead-ish, 
but I'd like to see a 'fourcc_t' typedef of the u32.

> 
> The format negotiation operation in the bridges could benefit too.
> 
>> +{
>> +	uint32_t *fourccs = fourccs_out;
>> +	const uint32_t *fourccs_end = fourccs_out + nfourccs_out;
>> +	bool found_native = false;
>> +	size_t nfourccs, i;
>> +
>> +	/* native formats go first */
>> +
> Drop extra line, capital start
>> +	nfourccs = min_t(size_t, native_nfourccs, nfourccs_out);
>> +
>> +	for (i = 0; i < nfourccs; ++i) {
>> +		uint32_t fourcc = native_fourccs[i];
>> +
>> +		drm_dbg_kms(dev, "adding native format %p4cc\n", &fourcc);
>> +
>> +		if (!found_native)
>> +			found_native = is_listed_fourcc(extra_fourccs, extra_nfourccs, fourcc);
>> +		*fourccs = fourcc;
>> +		++fourccs;
>> +	}
>> +
>> +	/*
>> +	 * The plane's atomic_update helper converts the framebuffer's color format
>> +	 * to the native format when copying them to device memory.
>> +	 *
>> +	 * If there is not a single format supported by both, device and
>> +	 * plane, the native formats are likely not supported by the conversion
>> +	 * helpers. Therefore *only* support the native formats and add a
>> +	 * conversion helper ASAP.
>> +	 */
> Despite the nice comment I had to think twice. In the end I agree with
> this.
> 
>> +	if (!found_native) {
>> +		drm_warn(dev, "format conversion helpers required to add extra formats\n");
>> +		goto out;
>> +	}
>> +
>> +	/* extra formats go second */
>> +
> Drop extra line, capital start.
>> +	nfourccs = min_t(size_t, extra_nfourccs, fourccs_end - fourccs);
>> +
>> +	for (i = 0; i < nfourccs; ++i) {
>> +		uint32_t fourcc = extra_fourccs[i];
>> +
>> +		if (is_listed_fourcc(native_fourccs, native_nfourccs, fourcc))
>> +			continue; /* native formats already went first */
>> +		*fourccs = fourcc;
>> +		++fourccs;
>> +	}
>> +
>> +out:
>> +	return fourccs - fourccs_out;
>> +}
> It would be prudent to warn if the supplied fourccs_out array is too
> small to the provided input formats. simpledrm is about to hit the limit.

Good idea. I've added a warning.

Best regards
Thomas

> 
>> +EXPORT_SYMBOL(drm_fb_build_fourcc_list);
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index cc509154b296..79c9fd6bedf0 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -644,45 +644,6 @@ static struct drm_display_mode simpledrm_mode(unsigned int width,
>>   	return mode;
>>   }
>>   
>> -static const uint32_t *simpledrm_device_formats(struct simpledrm_device *sdev,
>> -						size_t *nformats_out)
>> -{
>> -	struct drm_device *dev = &sdev->dev;
>> -	size_t i;
>> -
>> -	if (sdev->nformats)
>> -		goto out; /* don't rebuild list on recurring calls */
>> -
>> -	/* native format goes first */
>> -	sdev->formats[0] = sdev->format->format;
>> -	sdev->nformats = 1;
>> -
>> -	/* default formats go second */
>> -	for (i = 0; i < ARRAY_SIZE(simpledrm_primary_plane_formats); ++i) {
>> -		if (simpledrm_primary_plane_formats[i] == sdev->format->format)
>> -			continue; /* native format already went first */
>> -		sdev->formats[sdev->nformats] = simpledrm_primary_plane_formats[i];
>> -		sdev->nformats++;
>> -	}
>> -
>> -	/*
>> -	 * TODO: The simpledrm driver converts framebuffers to the native
>> -	 * format when copying them to device memory. If there are more
>> -	 * formats listed than supported by the driver, the native format
>> -	 * is not supported by the conversion helpers. Therefore *only*
>> -	 * support the native format and add a conversion helper ASAP.
>> -	 */
>> -	if (drm_WARN_ONCE(dev, i != sdev->nformats,
>> -			  "format conversion helpers required for %p4cc",
>> -			  &sdev->format->format)) {
>> -		sdev->nformats = 1;
>> -	}
>> -
>> -out:
>> -	*nformats_out = sdev->nformats;
>> -	return sdev->formats;
>> -}
>> -
>>   static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   							struct platform_device *pdev)
>>   {
>> @@ -699,7 +660,6 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   	struct drm_encoder *encoder;
>>   	struct drm_connector *connector;
>>   	unsigned long max_width, max_height;
>> -	const uint32_t *formats;
>>   	size_t nformats;
>>   	int ret;
>>   
>> @@ -811,11 +771,14 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   
>>   	/* Primary plane */
>>   
>> -	formats = simpledrm_device_formats(sdev, &nformats);
>> +	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
>> +					    simpledrm_primary_plane_formats,
>> +					    ARRAY_SIZE(simpledrm_primary_plane_formats),
>> +					    sdev->formats, ARRAY_SIZE(sdev->formats));
> simpledrm_primary_plane_formats is 6 long, with a todo to add 2 more.
> So the current array of 8 in sdev->formats is big enough for now.
> 
> 
>>   
>>   	primary_plane = &sdev->primary_plane;
>>   	ret = drm_universal_plane_init(dev, primary_plane, 0, &simpledrm_primary_plane_funcs,
>> -				       formats, nformats,
>> +				       sdev->formats, nformats,
>>   				       simpledrm_primary_plane_format_modifiers,
>>   				       DRM_PLANE_TYPE_PRIMARY, NULL);
>>   	if (ret)
>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>> index caa181194335..33278870e0d8 100644
>> --- a/include/drm/drm_format_helper.h
>> +++ b/include/drm/drm_format_helper.h
>> @@ -6,11 +6,15 @@
>>   #ifndef __LINUX_DRM_FORMAT_HELPER_H
>>   #define __LINUX_DRM_FORMAT_HELPER_H
>>   
>> -struct iosys_map;
>> +#include <linux/types.h>
>> +
>> +struct drm_device;
>>   struct drm_format_info;
>>   struct drm_framebuffer;
>>   struct drm_rect;
>>   
>> +struct iosys_map;
>> +
>>   unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
>>   				const struct drm_rect *clip);
>>   
>> @@ -44,4 +48,9 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>>   			     const struct iosys_map *src, const struct drm_framebuffer *fb,
>>   			     const struct drm_rect *clip);
>>   
>> +size_t drm_fb_build_fourcc_list(struct drm_device *dev,
>> +				const uint32_t *native_fourccs, size_t native_nfourccs,
>> +				const uint32_t *extra_fourccs, size_t extra_nfourccs,
>> +				uint32_t *fourccs_out, size_t nfourccs_out);
>> +
>>   #endif /* __LINUX_DRM_FORMAT_HELPER_H */
>> -- 
>> 2.37.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 56642816fdff..dca5531615f3 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -793,3 +793,97 @@  void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 	kfree(src32);
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
+
+static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs, uint32_t fourcc)
+{
+	const uint32_t *fourccs_end = fourccs + nfourccs;
+
+	while (fourccs < fourccs_end) {
+		if (*fourccs == fourcc)
+			return true;
+		++fourccs;
+	}
+	return false;
+}
+
+/**
+ * drm_fb_build_fourcc_list - Filters a list of supported color formats against
+ *                            the device's native formats
+ * @dev: DRM device
+ * @native_fourccs: 4CC codes of natively supported color formats
+ * @native_nfourccs: The number of entries in @native_fourccs
+ * @extra_fourccs: 4CC codes of additionally supported color formats
+ * @extra_nfourccs: The number of entries in @extra_fourccs
+ * @fourccs_out: Returns 4CC codes of supported color formats
+ * @nfourccs_out: The number of available entries in @fourccs_out
+ *
+ * This function create a list of supported color format from natively
+ * supported formats and the emulated formats.  *
+ * At a minimum, most userspace programs expect at least support for
+ * XRGB8888 on the primary plane. Devices that have to emulate the
+ * format, and possibly others, can use drm_fb_build_fourcc_list() to
+ * create a list of supported color formats. The returned list can
+ * be handed over to drm_universal_plane_init() et al. Native formats
+ * will go before emulated formats. Other heuristics might be applied
+ * to optimize the order. Formats near the beginning of the list are
+ * usually preferred over formats near the end of the list.
+ *
+ * Returns:
+ * The number of color-formats 4CC codes returned in @fourccs_out.
+ */
+size_t drm_fb_build_fourcc_list(struct drm_device *dev,
+				const uint32_t *native_fourccs, size_t native_nfourccs,
+				const uint32_t *extra_fourccs, size_t extra_nfourccs,
+				uint32_t *fourccs_out, size_t nfourccs_out)
+{
+	uint32_t *fourccs = fourccs_out;
+	const uint32_t *fourccs_end = fourccs_out + nfourccs_out;
+	bool found_native = false;
+	size_t nfourccs, i;
+
+	/* native formats go first */
+
+	nfourccs = min_t(size_t, native_nfourccs, nfourccs_out);
+
+	for (i = 0; i < nfourccs; ++i) {
+		uint32_t fourcc = native_fourccs[i];
+
+		drm_dbg_kms(dev, "adding native format %p4cc\n", &fourcc);
+
+		if (!found_native)
+			found_native = is_listed_fourcc(extra_fourccs, extra_nfourccs, fourcc);
+		*fourccs = fourcc;
+		++fourccs;
+	}
+
+	/*
+	 * The plane's atomic_update helper converts the framebuffer's color format
+	 * to the native format when copying them to device memory.
+	 *
+	 * If there is not a single format supported by both, device and
+	 * plane, the native formats are likely not supported by the conversion
+	 * helpers. Therefore *only* support the native formats and add a
+	 * conversion helper ASAP.
+	 */
+	if (!found_native) {
+		drm_warn(dev, "format conversion helpers required to add extra formats\n");
+		goto out;
+	}
+
+	/* extra formats go second */
+
+	nfourccs = min_t(size_t, extra_nfourccs, fourccs_end - fourccs);
+
+	for (i = 0; i < nfourccs; ++i) {
+		uint32_t fourcc = extra_fourccs[i];
+
+		if (is_listed_fourcc(native_fourccs, native_nfourccs, fourcc))
+			continue; /* native formats already went first */
+		*fourccs = fourcc;
+		++fourccs;
+	}
+
+out:
+	return fourccs - fourccs_out;
+}
+EXPORT_SYMBOL(drm_fb_build_fourcc_list);
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index cc509154b296..79c9fd6bedf0 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -644,45 +644,6 @@  static struct drm_display_mode simpledrm_mode(unsigned int width,
 	return mode;
 }
 
-static const uint32_t *simpledrm_device_formats(struct simpledrm_device *sdev,
-						size_t *nformats_out)
-{
-	struct drm_device *dev = &sdev->dev;
-	size_t i;
-
-	if (sdev->nformats)
-		goto out; /* don't rebuild list on recurring calls */
-
-	/* native format goes first */
-	sdev->formats[0] = sdev->format->format;
-	sdev->nformats = 1;
-
-	/* default formats go second */
-	for (i = 0; i < ARRAY_SIZE(simpledrm_primary_plane_formats); ++i) {
-		if (simpledrm_primary_plane_formats[i] == sdev->format->format)
-			continue; /* native format already went first */
-		sdev->formats[sdev->nformats] = simpledrm_primary_plane_formats[i];
-		sdev->nformats++;
-	}
-
-	/*
-	 * TODO: The simpledrm driver converts framebuffers to the native
-	 * format when copying them to device memory. If there are more
-	 * formats listed than supported by the driver, the native format
-	 * is not supported by the conversion helpers. Therefore *only*
-	 * support the native format and add a conversion helper ASAP.
-	 */
-	if (drm_WARN_ONCE(dev, i != sdev->nformats,
-			  "format conversion helpers required for %p4cc",
-			  &sdev->format->format)) {
-		sdev->nformats = 1;
-	}
-
-out:
-	*nformats_out = sdev->nformats;
-	return sdev->formats;
-}
-
 static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 							struct platform_device *pdev)
 {
@@ -699,7 +660,6 @@  static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	unsigned long max_width, max_height;
-	const uint32_t *formats;
 	size_t nformats;
 	int ret;
 
@@ -811,11 +771,14 @@  static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 
 	/* Primary plane */
 
-	formats = simpledrm_device_formats(sdev, &nformats);
+	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
+					    simpledrm_primary_plane_formats,
+					    ARRAY_SIZE(simpledrm_primary_plane_formats),
+					    sdev->formats, ARRAY_SIZE(sdev->formats));
 
 	primary_plane = &sdev->primary_plane;
 	ret = drm_universal_plane_init(dev, primary_plane, 0, &simpledrm_primary_plane_funcs,
-				       formats, nformats,
+				       sdev->formats, nformats,
 				       simpledrm_primary_plane_format_modifiers,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret)
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index caa181194335..33278870e0d8 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -6,11 +6,15 @@ 
 #ifndef __LINUX_DRM_FORMAT_HELPER_H
 #define __LINUX_DRM_FORMAT_HELPER_H
 
-struct iosys_map;
+#include <linux/types.h>
+
+struct drm_device;
 struct drm_format_info;
 struct drm_framebuffer;
 struct drm_rect;
 
+struct iosys_map;
+
 unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
 				const struct drm_rect *clip);
 
@@ -44,4 +48,9 @@  void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 			     const struct iosys_map *src, const struct drm_framebuffer *fb,
 			     const struct drm_rect *clip);
 
+size_t drm_fb_build_fourcc_list(struct drm_device *dev,
+				const uint32_t *native_fourccs, size_t native_nfourccs,
+				const uint32_t *extra_fourccs, size_t extra_nfourccs,
+				uint32_t *fourccs_out, size_t nfourccs_out);
+
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */