Re: [PATCH v2] drm/malidp: Fix writeback in NV12

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 22, 2018 at 04:18:19PM +0100, Alexandru Gheorghe wrote:
> When we want to writeback to memory in NV12 format we need to program
> the RGB2YUV coefficients. Currently, we don't program the coefficients
> and NV12 doesn't work at all.
> 
> This patchset fixes that by programming a sane default(bt709, limited
> range) as rgb2yuv coefficients.
> 
> In the long run, probably we need to think of a way for userspace to
> be able to program that, but for now I think this is better than not
> working at all or not advertising NV12 as a supported format for
> memwrite.
> 
> Changes since v1:
>  - Write the rgb2yuv coefficients only once, since we don't change
>    them at all, just write them the first time NV12 is programmed,
>    suggested by Brian Starkey, here [1]
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2018-August/186819.html
> 
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>

Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx>

> ---
>  drivers/gpu/drm/arm/malidp_hw.c   | 25 +++++++++++++++++++++++--
>  drivers/gpu/drm/arm/malidp_hw.h   |  3 ++-
>  drivers/gpu/drm/arm/malidp_mw.c   | 25 +++++++++++++++++++++----
>  drivers/gpu/drm/arm/malidp_regs.h |  2 ++
>  4 files changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index c94a4422e0e9..2781e462c1ed 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -384,7 +384,8 @@ static long malidp500_se_calc_mclk(struct malidp_hw_device *hwdev,
>  
>  static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev,
>  				     dma_addr_t *addrs, s32 *pitches,
> -				     int num_planes, u16 w, u16 h, u32 fmt_id)
> +				     int num_planes, u16 w, u16 h, u32 fmt_id,
> +				     const s16 *rgb2yuv_coeffs)
>  {
>  	u32 base = MALIDP500_SE_MEMWRITE_BASE;
>  	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
> @@ -416,6 +417,16 @@ static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev,
>  
>  	malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h),
>  			MALIDP500_SE_MEMWRITE_OUT_SIZE);
> +
> +	if (rgb2yuv_coeffs) {
> +		int i;
> +
> +		for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) {
> +			malidp_hw_write(hwdev, rgb2yuv_coeffs[i],
> +					MALIDP500_SE_RGB_YUV_COEFFS + i * 4);
> +		}
> +	}
> +
>  	malidp_hw_setbits(hwdev, MALIDP_SE_MEMWRITE_EN, MALIDP500_SE_CONTROL);
>  
>  	return 0;
> @@ -658,7 +669,8 @@ static long malidp550_se_calc_mclk(struct malidp_hw_device *hwdev,
>  
>  static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
>  				     dma_addr_t *addrs, s32 *pitches,
> -				     int num_planes, u16 w, u16 h, u32 fmt_id)
> +				     int num_planes, u16 w, u16 h, u32 fmt_id,
> +				     const s16 *rgb2yuv_coeffs)
>  {
>  	u32 base = MALIDP550_SE_MEMWRITE_BASE;
>  	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
> @@ -689,6 +701,15 @@ static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
>  	malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN,
>  			  MALIDP550_SE_CONTROL);
>  
> +	if (rgb2yuv_coeffs) {
> +		int i;
> +
> +		for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) {
> +			malidp_hw_write(hwdev, rgb2yuv_coeffs[i],
> +					MALIDP550_SE_RGB_YUV_COEFFS + i * 4);
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
> index ad2e96915d44..9fc94c08190f 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.h
> +++ b/drivers/gpu/drm/arm/malidp_hw.h
> @@ -191,7 +191,8 @@ struct malidp_hw {
>  	 * @param fmt_id - internal format ID of output buffer
>  	 */
>  	int (*enable_memwrite)(struct malidp_hw_device *hwdev, dma_addr_t *addrs,
> -			       s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id);
> +			       s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id,
> +			       const s16 *rgb2yuv_coeffs);
>  
>  	/*
>  	 * Disable the writing to memory of the next frame's content.
> diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
> index cfd718e7e97c..429a11e6473b 100644
> --- a/drivers/gpu/drm/arm/malidp_mw.c
> +++ b/drivers/gpu/drm/arm/malidp_mw.c
> @@ -26,6 +26,8 @@ struct malidp_mw_connector_state {
>  	s32 pitches[2];
>  	u8 format;
>  	u8 n_planes;
> +	bool rgb2yuv_initialized;
> +	const s16 *rgb2yuv_coeffs;
>  };
>  
>  static int malidp_mw_connector_get_modes(struct drm_connector *connector)
> @@ -84,7 +86,7 @@ static void malidp_mw_connector_destroy(struct drm_connector *connector)
>  static struct drm_connector_state *
>  malidp_mw_connector_duplicate_state(struct drm_connector *connector)
>  {
> -	struct malidp_mw_connector_state *mw_state;
> +	struct malidp_mw_connector_state *mw_state, *mw_current_state;
>  
>  	if (WARN_ON(!connector->state))
>  		return NULL;
> @@ -93,7 +95,10 @@ malidp_mw_connector_duplicate_state(struct drm_connector *connector)
>  	if (!mw_state)
>  		return NULL;
>  
> -	/* No need to preserve any of our driver-local data */
> +	mw_current_state = to_mw_state(connector->state);
> +	mw_state->rgb2yuv_coeffs = mw_current_state->rgb2yuv_coeffs;
> +	mw_state->rgb2yuv_initialized = mw_current_state->rgb2yuv_initialized;
> +
>  	__drm_atomic_helper_connector_duplicate_state(connector, &mw_state->base);
>  
>  	return &mw_state->base;
> @@ -108,6 +113,13 @@ static const struct drm_connector_funcs malidp_mw_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> +static const s16 rgb2yuv_coeffs_bt709_limited[MALIDP_COLORADJ_NUM_COEFFS] = {
> +	47,  157,   16,
> +	-26,  -87,  112,
> +	112, -102,  -10,
> +	16,  128,  128
> +};
> +
>  static int
>  malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
>  			       struct drm_crtc_state *crtc_state,
> @@ -157,6 +169,9 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
>  	}
>  	mw_state->n_planes = n_planes;
>  
> +	if (fb->format->is_yuv)
> +		mw_state->rgb2yuv_coeffs = rgb2yuv_coeffs_bt709_limited;
> +
>  	return 0;
>  }
>  
> @@ -239,10 +254,12 @@ void malidp_mw_atomic_commit(struct drm_device *drm,
>  
>  		drm_writeback_queue_job(mw_conn, conn_state->writeback_job);
>  		conn_state->writeback_job = NULL;
> -
>  		hwdev->hw->enable_memwrite(hwdev, mw_state->addrs,
>  					   mw_state->pitches, mw_state->n_planes,
> -					   fb->width, fb->height, mw_state->format);
> +					   fb->width, fb->height, mw_state->format,
> +					   !mw_state->rgb2yuv_initialized ?
> +					   mw_state->rgb2yuv_coeffs : NULL);
> +		mw_state->rgb2yuv_initialized = !!mw_state->rgb2yuv_coeffs;
>  	} else {
>  		DRM_DEV_DEBUG_DRIVER(drm->dev, "Disable memwrite\n");
>  		hwdev->hw->disable_memwrite(hwdev);
> diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
> index 3579d36b2a71..6ffe849774f2 100644
> --- a/drivers/gpu/drm/arm/malidp_regs.h
> +++ b/drivers/gpu/drm/arm/malidp_regs.h
> @@ -205,6 +205,7 @@
>  #define MALIDP500_SE_BASE		0x00c00
>  #define MALIDP500_SE_CONTROL		0x00c0c
>  #define MALIDP500_SE_MEMWRITE_OUT_SIZE	0x00c2c
> +#define MALIDP500_SE_RGB_YUV_COEFFS	0x00C74
>  #define MALIDP500_SE_MEMWRITE_BASE	0x00e00
>  #define MALIDP500_DC_IRQ_BASE		0x00f00
>  #define MALIDP500_CONFIG_VALID		0x00f00
> @@ -238,6 +239,7 @@
>  #define MALIDP550_SE_CONTROL		0x08010
>  #define   MALIDP550_SE_MEMWRITE_ONESHOT	(1 << 7)
>  #define MALIDP550_SE_MEMWRITE_OUT_SIZE	0x08030
> +#define MALIDP550_SE_RGB_YUV_COEFFS	0x08078
>  #define MALIDP550_SE_MEMWRITE_BASE	0x08100
>  #define MALIDP550_DC_BASE		0x0c000
>  #define MALIDP550_DC_CONTROL		0x0c010
> -- 
> 2.18.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux