Re: [PATCH 2/2] drm/stm: ltdc: add crtc background color property support

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5432 bytes --]

On Wed, 7 Jul 2021 08:48:55 +0000
Raphael GALLAIS-POU - foss <raphael.gallais-pou@foss.st.com> wrote:

> This patch comes from the need to display small resolution pictures with
> very few DDR usage. In practice, using a background color, produced by the
> drm CRTC, around this picture allows to fetch less data in memory than
> setting a full frame picture. And therefore the picture in DDR is smaller
> than the size of the screen.
> 
> It uses the DRM framework background color property and modifies the
> color to any value between 0x000000 and 0xFFFFFF from userland with a
> RGB24 value (0x00RRGGBB).
> 
> Using this feature is observable only if layers are not full screen
> or if layers use color formats with alpha and are "transparent" at
> least on some pixels.
> 
> Depending on the hardware version, the background color can not be
> properly displayed with non-alpha color formats derived from native
> alpha color formats (such as XR24 or XR15) since the use of this
> pixel format generates a non transparent layer. As a workaround,
> the stage background color of the layer and the general background
> color need to be synced.
> 
> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> ---
>  drivers/gpu/drm/stm/ltdc.c | 48 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 1f9392fb58e1..0aca245288cc 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -196,6 +196,11 @@
>  
>  #define NB_PF		8		/* Max nb of HW pixel format */
>  
> +#define DRM_ARGB_TO_LTDC_RGB24(bgcolor) \
> +	((u32)(DRM_ARGB_RED(bgcolor, 8) << 16	\
> +	| DRM_ARGB_GREEN(bgcolor, 8) << 8	\
> +	| DRM_ARGB_BLUE(bgcolor, 8)))
> +
>  enum ltdc_pix_fmt {
>  	PF_NONE,
>  	/* RGB formats */
> @@ -364,6 +369,15 @@ static inline u32 get_pixelformat_without_alpha(u32 drm)
>  	}
>  }
>  
> +/*
> + * All non-alpha color formats derived from native alpha color formats are
> + * either characterized by a FourCC format code (such as XR24, RX24, BX24...)
> + */
> +static inline u32 is_xrgb(u32 drm)
> +{
> +	return ((drm & 'X') == 'X' || (drm & ('X' << 8)) == ('X' << 8));

Hi,

if you're trying to test whether the last or second last byte in the
format code is exactly 'X', this doesn't do that. What this does is
ignores all the bits that are zero in 'X' and ensures that all the bits
that are one in 'X' are also one in the tested value.


Thanks,
pq

> +}
> +
>  static irqreturn_t ltdc_irq_thread(int irq, void *arg)
>  {
>  	struct drm_device *ddev = arg;
> @@ -431,7 +445,8 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc,
>  	pm_runtime_get_sync(ddev->dev);
>  
>  	/* Sets the background color value */
> -	reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
> +	reg_write(ldev->regs, LTDC_BCCR,
> +		  DRM_ARGB_TO_LTDC_RGB24(crtc->state->bgcolor));
>  
>  	/* Enable IRQ */
>  	reg_set(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
> @@ -452,6 +467,9 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc,
>  
>  	drm_crtc_vblank_off(crtc);
>  
> +	/* Reset background color */
> +	reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
> +
>  	/* disable IRQ */
>  	reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
>  
> @@ -790,6 +808,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
>  	u32 y1 = newstate->crtc_y + newstate->crtc_h - 1;
>  	u32 src_x, src_y, src_w, src_h;
>  	u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr;
> +	u32 bgcolor = DRM_ARGB_TO_LTDC_RGB24(newstate->crtc->state->bgcolor);
>  	enum ltdc_pix_fmt pf;
>  
>  	if (!newstate->crtc || !fb) {
> @@ -853,10 +872,28 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
>  	if (!fb->format->has_alpha)
>  		val = BF1_CA | BF2_1CA;
>  
> -	/* Manage hw-specific capabilities */
> -	if (ldev->caps.non_alpha_only_l1 &&
> -	    plane->type != DRM_PLANE_TYPE_PRIMARY)
> -		val = BF1_PAXCA | BF2_1PAXCA;
> +	/*
> +	 * Manage hw-specific capabilities
> +	 *
> +	 * Depending on the hardware version, the background color can not be
> +	 * properly displayed with non-alpha color formats derived from native
> +	 * alpha color formats (such as XR24 or XR15) since the use of this
> +	 * pixel format generates a non transparent layer. As a workaround,
> +	 * the stage background color of the layer and the general background
> +	 * color need to be synced.
> +	 *
> +	 * This is done by activating for all XRGB color format the default
> +	 * color as the background color and then setting blending factor
> +	 * accordingly.
> +	 */
> +	if (ldev->caps.non_alpha_only_l1) {
> +		if (is_xrgb(fb->format->format)) {
> +			val = BF1_CA | BF2_1CA;
> +			reg_write(ldev->regs, LTDC_L1DCCR + lofs, bgcolor);
> +		} else {
> +			val = BF1_PAXCA | BF2_1PAXCA;
> +		}
> +	}
>  
>  	reg_update_bits(ldev->regs, LTDC_L1BFCR + lofs,
>  			LXBFCR_BF2 | LXBFCR_BF1, val);
> @@ -1033,6 +1070,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
>  
>  	drm_crtc_helper_add(crtc, &ltdc_crtc_helper_funcs);
>  
> +	drm_crtc_add_bgcolor_property(crtc);
>  	drm_mode_crtc_set_gamma_size(crtc, CLUT_SIZE);
>  	drm_crtc_enable_color_mgmt(crtc, 0, false, CLUT_SIZE);
>  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]