drm/rockchip: Add AFBC support
diff mbox series

Message ID 20190923122014.18229-1-andrzej.p@collabora.com
State New
Headers show
Series
  • drm/rockchip: Add AFBC support
Related show

Commit Message

Andrzej Pietrasiewicz Sept. 23, 2019, 12:20 p.m. UTC
From: Ezequiel Garcia <ezequiel@collabora.com>

AFBC is a proprietary lossless image compression protocol and format.
It helps reduce memory bandwidth of the graphics pipeline operations.
This, in turn, improves power efficiency.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
[locking improvements]
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
[squashing the above, commit message and Rockchip AFBC modifier]
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
 include/uapi/drm/drm_fourcc.h               |  3 +
 5 files changed, 151 insertions(+), 3 deletions(-)

Comments

Neil Armstrong Sept. 23, 2019, 1:53 p.m. UTC | #1
On 23/09/2019 14:20, Andrzej Pietrasiewicz wrote:
> From: Ezequiel Garcia <ezequiel@collabora.com>
> 
> AFBC is a proprietary lossless image compression protocol and format.
> It helps reduce memory bandwidth of the graphics pipeline operations.
> This, in turn, improves power efficiency.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> [locking improvements]
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
>  include/uapi/drm/drm_fourcc.h               |  3 +
>  5 files changed, 151 insertions(+), 3 deletions(-)
> 

[...]

> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..ba6caf06c824 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,9 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>  
> +#define AFBC_FORMAT_MOD_ROCKCHIP \
> +	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)

This define looks useless, what's Rockchip specific here ?

Neil

> +
>  /*
>   * Allwinner tiled modifier
>   *
>
Andrzej Pietrasiewicz Sept. 23, 2019, 3:34 p.m. UTC | #2
Dear All,

As a result of my mistake I've sent this patch with an incorrect SOB chain. 
Please kindly disregard this patch.

@Neil: thank you for your time you spent reviewing it and answering and I'm 
sorry it's to no effect.
@Ezequiel, @Tomeu: I apologize to you. My mistake.

Regards,

Andrzej Pietrasiewicz


W dniu 23.09.2019 o 15:53, Neil Armstrong pisze:
> On 23/09/2019 14:20, Andrzej Pietrasiewicz wrote:
>> From: Ezequiel Garcia <ezequiel@collabora.com>
>>
>> AFBC is a proprietary lossless image compression protocol and format.
>> It helps reduce memory bandwidth of the graphics pipeline operations.
>> This, in turn, improves power efficiency.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> [locking improvements]
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> [squashing the above, commit message and Rockchip AFBC modifier]
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
>>   include/uapi/drm/drm_fourcc.h               |  3 +
>>   5 files changed, 151 insertions(+), 3 deletions(-)
>>
> 
> [...]
> 
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 3feeaa3f987a..ba6caf06c824 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -742,6 +742,9 @@ extern "C" {
>>    */
>>   #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>>   
>> +#define AFBC_FORMAT_MOD_ROCKCHIP \
>> +	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
> 
> This define looks useless, what's Rockchip specific here ?
> 
> Neil
> 
>> +
>>   /*
>>    * Allwinner tiled modifier
>>    *
>>
>
Ayan Halder Sept. 25, 2019, 9:28 a.m. UTC | #3
On Mon, Sep 23, 2019 at 05:34:14PM +0200, Andrzej Pietrasiewicz wrote:
> Dear All,
> 
> As a result of my mistake I've sent this patch with an incorrect SOB chain.
> Please kindly disregard this patch.
> 
> @Neil: thank you for your time you spent reviewing it and answering and I'm
> sorry it's to no effect.
> @Ezequiel, @Tomeu: I apologize to you. My mistake.
> 
> Regards,
> 
> Andrzej Pietrasiewicz
> 
> 
> W dniu 23.09.2019 o 15:53, Neil Armstrong pisze:
> >On 23/09/2019 14:20, Andrzej Pietrasiewicz wrote:
> >>From: Ezequiel Garcia <ezequiel@collabora.com>
> >>
> >>AFBC is a proprietary lossless image compression protocol and format.
> >>It helps reduce memory bandwidth of the graphics pipeline operations.
> >>This, in turn, improves power efficiency.
> >>
> >>Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> >>[locking improvements]
> >>Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> >>[squashing the above, commit message and Rockchip AFBC modifier]
> >>Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> >>---
> >>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
> >>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
> >>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
> >>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
> >>  include/uapi/drm/drm_fourcc.h               |  3 +
> >>  5 files changed, 151 insertions(+), 3 deletions(-)
> >>
> >
> >[...]
> >
> >>diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >>index 3feeaa3f987a..ba6caf06c824 100644
> >>--- a/include/uapi/drm/drm_fourcc.h
> >>+++ b/include/uapi/drm/drm_fourcc.h
> >>@@ -742,6 +742,9 @@ extern "C" {
> >>   */
> >>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
> >>+#define AFBC_FORMAT_MOD_ROCKCHIP \
> >>+	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
> >
> >This define looks useless, what's Rockchip specific here ?
> >
Please reuse the existing AFBC modifiers.

Have a look at malidp_format_modifiers[] in which we have defined
the modifiers(for our driver) we are using.

In your case, it will be
DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16|AFBC_FORMAT_MOD_SPARSE)

> >Neil
> >
> >>+
> >>  /*
> >>   * Allwinner tiled modifier
> >>   *
> >>
> >
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ayan Halder Sept. 25, 2019, 9:35 a.m. UTC | #4
On Mon, Sep 23, 2019 at 02:20:13PM +0200, Andrzej Pietrasiewicz wrote:
> From: Ezequiel Garcia <ezequiel@collabora.com>
> 
> AFBC is a proprietary lossless image compression protocol and format.
> It helps reduce memory bandwidth of the graphics pipeline operations.
> This, in turn, improves power efficiency.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> [locking improvements]
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
>  include/uapi/drm/drm_fourcc.h               |  3 +
>  5 files changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 64ca87cf6d50..5178939a9c29 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -24,6 +24,27 @@ static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>  	.dirty	       = drm_atomic_helper_dirtyfb,
>  };
>  
> +static int
> +rockchip_verify_afbc_mod(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	if (mode_cmd->modifier[0] &
> +	    ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "Unsupported format modifier 0x%llx\n",
> +			      mode_cmd->modifier[0]);
> +		return -EINVAL;
> +	}
> +
> +	if (mode_cmd->width > 2560) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "Unsupported width %d\n", mode_cmd->width);
> +		return -EINVAL;
> +	}
> +
I think you are missing one additional check here.
framebuffer width and height should be aligned to 16 pixels as you are
using AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
Please have a look at malidp_verify_afbc_framebuffer_caps()

> +	return 0;
> +}
> +
>  static struct drm_framebuffer *
>  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd,
>  		  struct drm_gem_object **obj, unsigned int num_planes)
> @@ -32,6 +53,12 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
>  	int ret;
>  	int i;
>  
> +	if (mode_cmd->modifier[0]) {
> +		ret = rockchip_verify_afbc_mod(dev, mode_cmd);
> +		if (ret)
> +			return ERR_PTR(ret);
> +	}
> +
>  	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>  	if (!fb)
>  		return ERR_PTR(-ENOMEM);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 21b68eea46cc..50bf214d21da 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -46,6 +46,13 @@
>  		vop_reg_set(vop, &win->phy->scl->ext->name, \
>  			    win->base, ~0, v, #name)
>  
> +#define VOP_AFBC_SET(x, name, v) \
> +	do { \
> +		if (vop->data->afbc) \
> +			vop_reg_set(vop, &vop->data->afbc->name, \
> +				0, ~0, v, #name); \
> +	} while (0)
> +
>  #define VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, name, v) \
>  	do { \
>  		if (win_yuv2yuv && win_yuv2yuv->name.mask) \
> @@ -123,6 +130,8 @@ struct vop {
>  	struct drm_device *drm_dev;
>  	bool is_enabled;
>  
> +	struct vop_win *afbc_win;
> +
>  	struct completion dsp_hold_completion;
>  
>  	/* protected by dev->event_lock */
> @@ -245,6 +254,30 @@ static bool has_rb_swapped(uint32_t format)
>  	}
>  }
>  
> +#define AFBC_FMT_RGB565        0x0
> +#define AFBC_FMT_U8U8U8U8      0x5
> +#define AFBC_FMT_U8U8U8        0x4
> +
> +static int vop_convert_afbc_format(uint32_t format)
> +{
> +	switch (format) {
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ABGR8888:
> +		return AFBC_FMT_U8U8U8U8;
> +	case DRM_FORMAT_RGB888:
> +	case DRM_FORMAT_BGR888:
> +		return AFBC_FMT_U8U8U8;
> +	case DRM_FORMAT_RGB565:
> +	case DRM_FORMAT_BGR565:
> +		return AFBC_FMT_RGB565;
> +	default:
> +		DRM_ERROR("unsupported afbc format[%08x]\n", format);
> +		return -EINVAL;
> +	}
> +}
> +
>  static enum vop_data_format vop_convert_format(uint32_t format)
>  {
>  	switch (format) {
> @@ -587,10 +620,16 @@ static int vop_enable(struct drm_crtc *crtc)
>  
>  		vop_win_disable(vop, win);
>  	}
> -	spin_unlock(&vop->reg_lock);
> +
> +	if (vop->data->afbc) {
> +		VOP_AFBC_SET(vop, enable, 0);
> +		vop->afbc_win = NULL;
> +	}
>  
>  	vop_cfg_done(vop);
>  
> +	spin_unlock(&vop->reg_lock);
> +
>  	/*
>  	 * At here, vop clock & iommu is enable, R/W vop regs would be safe.
>  	 */
> @@ -719,6 +758,32 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		struct vop *vop = to_vop(crtc);
> +
> +		if (!vop->data->afbc) {
> +			DRM_ERROR("VOP does not support AFBC\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = vop_convert_afbc_format(fb->format->format);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (state->src.x1 || state->src.y1) {
> +			DRM_ERROR("AFBC does not support offset display\n");
> +			DRM_ERROR("xpos=%d, ypos=%d, offset=%d\n",
> +				state->src.x1, state->src.y1, fb->offsets[0]);
> +			return -EINVAL;
> +		}
> +
> +		if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
> +			DRM_ERROR("AFBC does not support rotation\n");
> +			DRM_ERROR("rotation=%d\n", state->rotation);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -732,6 +797,9 @@ static void vop_plane_atomic_disable(struct drm_plane *plane,
>  	if (!old_state->crtc)
>  		return;
>  
> +	if (vop->afbc_win == vop_win)
> +		vop->afbc_win = NULL;
> +
>  	spin_lock(&vop->reg_lock);
>  
>  	vop_win_disable(vop, win);
> @@ -774,6 +842,9 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  	if (WARN_ON(!vop->is_enabled))
>  		return;
>  
> +	if (vop->afbc_win == vop_win)
> +		vop->afbc_win = NULL;
> +
>  	if (!state->visible) {
>  		vop_plane_atomic_disable(plane, old_state);
>  		return;
> @@ -808,6 +879,20 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  
>  	spin_lock(&vop->reg_lock);
>  
> +	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> +		VOP_AFBC_SET(vop, format, afbc_format | 1 << 4);
> +		VOP_AFBC_SET(vop, hreg_block_split, 0);
> +		VOP_AFBC_SET(vop, win_sel, win_index);
> +		VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> +		VOP_AFBC_SET(vop, pic_size, act_info);
> +
> +		vop->afbc_win = vop_win;
> +
> +		pr_info("AFBC on plane %s\n", plane->name);
> +	}
> +
>  	VOP_WIN_SET(vop, win, format, format);
>  	VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
>  	VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> @@ -1163,6 +1248,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>  
>  	spin_lock(&vop->reg_lock);
>  
> +	VOP_AFBC_SET(vop, enable, vop->afbc_win ? 0x1 : 0);
>  	vop_cfg_done(vop);
>  
>  	spin_unlock(&vop->reg_lock);
> @@ -1471,7 +1557,8 @@ static int vop_create_crtc(struct vop *vop)
>  					       0, &vop_plane_funcs,
>  					       win_data->phy->data_formats,
>  					       win_data->phy->nformats,
> -					       NULL, win_data->type, NULL);
> +					       win_data->phy->format_modifiers,
> +					       win_data->type, NULL);
>  		if (ret) {
>  			DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
>  				      ret);
> @@ -1511,7 +1598,8 @@ static int vop_create_crtc(struct vop *vop)
>  					       &vop_plane_funcs,
>  					       win_data->phy->data_formats,
>  					       win_data->phy->nformats,
> -					       NULL, win_data->type, NULL);
> +					       win_data->phy->format_modifiers,
> +					       win_data->type, NULL);
>  		if (ret) {
>  			DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
>  				      ret);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 2149a889c29d..dc8b12025269 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -77,6 +77,16 @@ struct vop_misc {
>  	struct vop_reg global_regdone_en;
>  };
>  
> +struct vop_afbc {
> +	struct vop_reg enable;
> +	struct vop_reg win_sel;
> +	struct vop_reg format;
> +	struct vop_reg hreg_block_split;
> +	struct vop_reg pic_size;
> +	struct vop_reg hdr_ptr;
> +	struct vop_reg rstn;
> +};
> +
>  struct vop_intr {
>  	const int *intrs;
>  	uint32_t nintrs;
> @@ -128,6 +138,7 @@ struct vop_win_phy {
>  	const struct vop_scl_regs *scl;
>  	const uint32_t *data_formats;
>  	uint32_t nformats;
> +	const uint64_t *format_modifiers;
>  
>  	struct vop_reg enable;
>  	struct vop_reg gate;
> @@ -169,6 +180,7 @@ struct vop_data {
>  	const struct vop_output *output;
>  	const struct vop_win_yuv2yuv_data *win_yuv2yuv;
>  	const struct vop_win_data *win;
> +	const struct vop_afbc *afbc;
>  	unsigned int win_size;
>  
>  #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 7b9c74750f6d..e9ff0c43c396 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -30,6 +30,12 @@
>  #define VOP_REG_MASK_SYNC(off, _mask, _shift) \
>  		_VOP_REG(off, _mask, _shift, true, false)
>  
> +static const uint64_t format_modifiers_afbc[] = {
> +	DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP),
> +	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_INVALID
> +};
> +
>  static const uint32_t formats_win_full[] = {
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_ARGB8888,
> @@ -667,6 +673,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>  	.scl = &rk3288_win_full_scl,
>  	.data_formats = formats_win_full,
>  	.nformats = ARRAY_SIZE(formats_win_full),
> +	.format_modifiers = format_modifiers_afbc,
>  	.enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
>  	.format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
>  	.rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
> @@ -758,6 +765,16 @@ static const struct vop_data rk3366_vop = {
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
>  };
>  
> +static const struct vop_afbc rk3399_afbc = {
> +	.rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
> +	.enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
> +	.win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
> +	.format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
> +	.hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
> +	.hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
> +	.pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
> +};
> +
>  static const struct vop_output rk3399_output = {
>  	.dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
>  	.rgb_pin_pol = VOP_REG(RK3368_DSP_CTRL1, 0xf, 16),
> @@ -808,6 +825,7 @@ static const struct vop_data rk3399_vop_big = {
>  	.modeset = &rk3288_modeset,
>  	.output = &rk3399_output,
>  	.misc = &rk3368_misc,
> +	.afbc = &rk3399_afbc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
>  	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..ba6caf06c824 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,9 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>  
> +#define AFBC_FORMAT_MOD_ROCKCHIP \
> +	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
> +
>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Brian Starkey Sept. 25, 2019, 9:39 a.m. UTC | #5
Hi Andrzej,

Thanks for the patch, it's nice to see another AFBC implementation
coming in.

For future versions, could you please Cc ayan.halder@arm.com? It would
have been nice to have someone @arm.com on patches which use/impact
Arm modifiers. Sadly I don't know how to make get_maintainer.pl help
with that.

Some more comments below.

On Mon, Sep 23, 2019 at 02:20:13PM +0200, Andrzej Pietrasiewicz wrote:
> From: Ezequiel Garcia <ezequiel@collabora.com>
> 
> AFBC is a proprietary lossless image compression protocol and format.
> It helps reduce memory bandwidth of the graphics pipeline operations.
> This, in turn, improves power efficiency.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> [locking improvements]
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
>  include/uapi/drm/drm_fourcc.h               |  3 +
>  5 files changed, 151 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 64ca87cf6d50..5178939a9c29 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -24,6 +24,27 @@ static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>  	.dirty	       = drm_atomic_helper_dirtyfb,
>  };
>  
> +static int
> +rockchip_verify_afbc_mod(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	if (mode_cmd->modifier[0] &
> +	    ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "Unsupported format modifier 0x%llx\n",
> +			      mode_cmd->modifier[0]);
> +		return -EINVAL;
> +	}
> +
> +	if (mode_cmd->width > 2560) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "Unsupported width %d\n", mode_cmd->width);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct drm_framebuffer *
>  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd,
>  		  struct drm_gem_object **obj, unsigned int num_planes)
> @@ -32,6 +53,12 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
>  	int ret;
>  	int i;
>  
> +	if (mode_cmd->modifier[0]) {
> +		ret = rockchip_verify_afbc_mod(dev, mode_cmd);
> +		if (ret)
> +			return ERR_PTR(ret);
> +	}
> +
>  	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>  	if (!fb)
>  		return ERR_PTR(-ENOMEM);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 21b68eea46cc..50bf214d21da 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -46,6 +46,13 @@
>  		vop_reg_set(vop, &win->phy->scl->ext->name, \
>  			    win->base, ~0, v, #name)
>  
> +#define VOP_AFBC_SET(x, name, v) \
> +	do { \
> +		if (vop->data->afbc) \
> +			vop_reg_set(vop, &vop->data->afbc->name, \
> +				0, ~0, v, #name); \
> +	} while (0)
> +
>  #define VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, name, v) \
>  	do { \
>  		if (win_yuv2yuv && win_yuv2yuv->name.mask) \
> @@ -123,6 +130,8 @@ struct vop {
>  	struct drm_device *drm_dev;
>  	bool is_enabled;
>  
> +	struct vop_win *afbc_win;
> +
>  	struct completion dsp_hold_completion;
>  
>  	/* protected by dev->event_lock */
> @@ -245,6 +254,30 @@ static bool has_rb_swapped(uint32_t format)
>  	}
>  }
>  
> +#define AFBC_FMT_RGB565        0x0
> +#define AFBC_FMT_U8U8U8U8      0x5
> +#define AFBC_FMT_U8U8U8        0x4
> +
> +static int vop_convert_afbc_format(uint32_t format)
> +{

It would be great if you are able to follow the guidance Arm published
here: https://www.kernel.org/doc/html/latest/gpu/afbc.html, which will
help ensure interoperability and compatibility between different
devices/drivers. Hopefully it doesn't limit some use-cases for you -
if it does, let's discuss them.

Specifically, please take a look at the format list there. Some of
your formats below are not on the preferred interop list:

> +	switch (format) {
> +	case DRM_FORMAT_XRGB8888:

XRGB8888: Not preferred, as encoding the X channel is a waste of bits

> +	case DRM_FORMAT_ARGB8888:

ARGB8888: Not preferred as the channel order prevents YTR

> +	case DRM_FORMAT_XBGR8888:

XBGR8888: Not preferred, as encoding the X channel is a waste of bits

> +	case DRM_FORMAT_ABGR8888:
> +		return AFBC_FMT_U8U8U8U8;
> +	case DRM_FORMAT_RGB888:

RGB888: Not preferred as the channel order prevents YTR

> +	case DRM_FORMAT_BGR888:
> +		return AFBC_FMT_U8U8U8;
> +	case DRM_FORMAT_RGB565:

RGB565: Not preferred as the channel order prevents YTR

> +	case DRM_FORMAT_BGR565:
> +		return AFBC_FMT_RGB565;
> +	default:
> +		DRM_ERROR("unsupported afbc format[%08x]\n", format);
> +		return -EINVAL;
> +	}
> +}
> +
>  static enum vop_data_format vop_convert_format(uint32_t format)
>  {
>  	switch (format) {
> @@ -587,10 +620,16 @@ static int vop_enable(struct drm_crtc *crtc)
>  
>  		vop_win_disable(vop, win);
>  	}
> -	spin_unlock(&vop->reg_lock);
> +
> +	if (vop->data->afbc) {
> +		VOP_AFBC_SET(vop, enable, 0);
> +		vop->afbc_win = NULL;
> +	}
>  
>  	vop_cfg_done(vop);
>  
> +	spin_unlock(&vop->reg_lock);
> +
>  	/*
>  	 * At here, vop clock & iommu is enable, R/W vop regs would be safe.
>  	 */
> @@ -719,6 +758,32 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		struct vop *vop = to_vop(crtc);
> +
> +		if (!vop->data->afbc) {
> +			DRM_ERROR("VOP does not support AFBC\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = vop_convert_afbc_format(fb->format->format);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (state->src.x1 || state->src.y1) {
> +			DRM_ERROR("AFBC does not support offset display\n");
> +			DRM_ERROR("xpos=%d, ypos=%d, offset=%d\n",
> +				state->src.x1, state->src.y1, fb->offsets[0]);
> +			return -EINVAL;
> +		}
> +
> +		if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
> +			DRM_ERROR("AFBC does not support rotation\n");
> +			DRM_ERROR("rotation=%d\n", state->rotation);
> +			return -EINVAL;
> +		}

It may be a good idea to check your framebuffer size, as the required
framebuffer size is different for AFBC. You can refer to mali-dp for
the calculations - perhaps even share the code.

> +	}
> +
>  	return 0;
>  }
>  
> @@ -732,6 +797,9 @@ static void vop_plane_atomic_disable(struct drm_plane *plane,
>  	if (!old_state->crtc)
>  		return;
>  
> +	if (vop->afbc_win == vop_win)
> +		vop->afbc_win = NULL;
> +
>  	spin_lock(&vop->reg_lock);
>  
>  	vop_win_disable(vop, win);
> @@ -774,6 +842,9 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  	if (WARN_ON(!vop->is_enabled))
>  		return;
>  
> +	if (vop->afbc_win == vop_win)
> +		vop->afbc_win = NULL;
> +
>  	if (!state->visible) {
>  		vop_plane_atomic_disable(plane, old_state);
>  		return;
> @@ -808,6 +879,20 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  
>  	spin_lock(&vop->reg_lock);
>  
> +	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> +		int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> +		VOP_AFBC_SET(vop, format, afbc_format | 1 << 4);
> +		VOP_AFBC_SET(vop, hreg_block_split, 0);
> +		VOP_AFBC_SET(vop, win_sel, win_index);
> +		VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> +		VOP_AFBC_SET(vop, pic_size, act_info);
> +
> +		vop->afbc_win = vop_win;
> +
> +		pr_info("AFBC on plane %s\n", plane->name);
> +	}
> +
>  	VOP_WIN_SET(vop, win, format, format);
>  	VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
>  	VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> @@ -1163,6 +1248,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>  
>  	spin_lock(&vop->reg_lock);
>  
> +	VOP_AFBC_SET(vop, enable, vop->afbc_win ? 0x1 : 0);
>  	vop_cfg_done(vop);
>  
>  	spin_unlock(&vop->reg_lock);
> @@ -1471,7 +1557,8 @@ static int vop_create_crtc(struct vop *vop)
>  					       0, &vop_plane_funcs,
>  					       win_data->phy->data_formats,
>  					       win_data->phy->nformats,
> -					       NULL, win_data->type, NULL);
> +					       win_data->phy->format_modifiers,
> +					       win_data->type, NULL);
>  		if (ret) {
>  			DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
>  				      ret);
> @@ -1511,7 +1598,8 @@ static int vop_create_crtc(struct vop *vop)
>  					       &vop_plane_funcs,
>  					       win_data->phy->data_formats,
>  					       win_data->phy->nformats,
> -					       NULL, win_data->type, NULL);
> +					       win_data->phy->format_modifiers,
> +					       win_data->type, NULL);
>  		if (ret) {
>  			DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
>  				      ret);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 2149a889c29d..dc8b12025269 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -77,6 +77,16 @@ struct vop_misc {
>  	struct vop_reg global_regdone_en;
>  };
>  
> +struct vop_afbc {
> +	struct vop_reg enable;
> +	struct vop_reg win_sel;
> +	struct vop_reg format;
> +	struct vop_reg hreg_block_split;
> +	struct vop_reg pic_size;
> +	struct vop_reg hdr_ptr;
> +	struct vop_reg rstn;
> +};
> +
>  struct vop_intr {
>  	const int *intrs;
>  	uint32_t nintrs;
> @@ -128,6 +138,7 @@ struct vop_win_phy {
>  	const struct vop_scl_regs *scl;
>  	const uint32_t *data_formats;
>  	uint32_t nformats;
> +	const uint64_t *format_modifiers;
>  
>  	struct vop_reg enable;
>  	struct vop_reg gate;
> @@ -169,6 +180,7 @@ struct vop_data {
>  	const struct vop_output *output;
>  	const struct vop_win_yuv2yuv_data *win_yuv2yuv;
>  	const struct vop_win_data *win;
> +	const struct vop_afbc *afbc;
>  	unsigned int win_size;
>  
>  #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 7b9c74750f6d..e9ff0c43c396 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -30,6 +30,12 @@
>  #define VOP_REG_MASK_SYNC(off, _mask, _shift) \
>  		_VOP_REG(off, _mask, _shift, true, false)
>  
> +static const uint64_t format_modifiers_afbc[] = {
> +	DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP),
> +	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_INVALID
> +};
> +
>  static const uint32_t formats_win_full[] = {
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_ARGB8888,
> @@ -667,6 +673,7 @@ static const struct vop_win_phy rk3368_win01_data = {
>  	.scl = &rk3288_win_full_scl,
>  	.data_formats = formats_win_full,
>  	.nformats = ARRAY_SIZE(formats_win_full),
> +	.format_modifiers = format_modifiers_afbc,

I may have missed something, but don't you need to implement the
format_mod_supported hook on the plane to expose the modifiers to
userspace?

>  	.enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
>  	.format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
>  	.rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
> @@ -758,6 +765,16 @@ static const struct vop_data rk3366_vop = {
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
>  };
>  
> +static const struct vop_afbc rk3399_afbc = {
> +	.rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
> +	.enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
> +	.win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
> +	.format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
> +	.hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
> +	.hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
> +	.pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
> +};
> +
>  static const struct vop_output rk3399_output = {
>  	.dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
>  	.rgb_pin_pol = VOP_REG(RK3368_DSP_CTRL1, 0xf, 16),
> @@ -808,6 +825,7 @@ static const struct vop_data rk3399_vop_big = {
>  	.modeset = &rk3288_modeset,
>  	.output = &rk3399_output,
>  	.misc = &rk3368_misc,
> +	.afbc = &rk3399_afbc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
>  	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..ba6caf06c824 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,9 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>  
> +#define AFBC_FORMAT_MOD_ROCKCHIP \
> +	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
> +

As Neil said, this seems redundant. If you would like the convenience
macro, stick it in the Rockchip driver rather than UAPI.

Also, if your hardware is able to support YTR, I would recommend
enabling it.

Best regards,
-Brian

>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Andrzej Pietrasiewicz Sept. 25, 2019, 9:42 a.m. UTC | #6
Hi Brian,

W dniu 25.09.2019 o 11:39, Brian Starkey pisze:
> Hi Andrzej,
> 
> Thanks for the patch, it's nice to see another AFBC implementation
> coming in.
> 

I did a false start, though. But the work is in progress. Thanks for the review, 
anyway.

> For future versions, could you please Cc ayan.halder@arm.com? It would
> have been nice to have someone @arm.com on patches which use/impact
> Arm modifiers. Sadly I don't know how to make get_maintainer.pl help

Of course.

Andrzej
Daniel Vetter Sept. 25, 2019, 11:12 a.m. UTC | #7
On Wed, Sep 25, 2019 at 11:35 AM Ayan Halder <Ayan.Halder@arm.com> wrote:
>
> On Mon, Sep 23, 2019 at 02:20:13PM +0200, Andrzej Pietrasiewicz wrote:
> > From: Ezequiel Garcia <ezequiel@collabora.com>
> >
> > AFBC is a proprietary lossless image compression protocol and format.
> > It helps reduce memory bandwidth of the graphics pipeline operations.
> > This, in turn, improves power efficiency.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > [locking improvements]
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > [squashing the above, commit message and Rockchip AFBC modifier]
> > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> > ---
> >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 27 ++++++
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++++++++++++++++++-
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 +++
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 ++++
> >  include/uapi/drm/drm_fourcc.h               |  3 +
> >  5 files changed, 151 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > index 64ca87cf6d50..5178939a9c29 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > @@ -24,6 +24,27 @@ static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> >       .dirty         = drm_atomic_helper_dirtyfb,
> >  };
> >
> > +static int
> > +rockchip_verify_afbc_mod(struct drm_device *dev,
> > +                      const struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > +     if (mode_cmd->modifier[0] &
> > +         ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                           "Unsupported format modifier 0x%llx\n",
> > +                           mode_cmd->modifier[0]);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (mode_cmd->width > 2560) {
> > +             DRM_DEV_ERROR(dev->dev,
> > +                           "Unsupported width %d\n", mode_cmd->width);
> > +             return -EINVAL;
> > +     }
> > +
> I think you are missing one additional check here.
> framebuffer width and height should be aligned to 16 pixels as you are
> using AFBC_FORMAT_MOD_BLOCK_SIZE_16x16.
> Please have a look at malidp_verify_afbc_framebuffer_caps()

Maybe time to extract that into a helper, so that there's only one
copy of the code that validates a buffer with afbc modifier set?
-Daniel

>
> > +     return 0;
> > +}
> > +
> >  static struct drm_framebuffer *
> >  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd,
> >                 struct drm_gem_object **obj, unsigned int num_planes)
> > @@ -32,6 +53,12 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
> >       int ret;
> >       int i;
> >
> > +     if (mode_cmd->modifier[0]) {
> > +             ret = rockchip_verify_afbc_mod(dev, mode_cmd);
> > +             if (ret)
> > +                     return ERR_PTR(ret);
> > +     }
> > +
> >       fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> >       if (!fb)
> >               return ERR_PTR(-ENOMEM);
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index 21b68eea46cc..50bf214d21da 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -46,6 +46,13 @@
> >               vop_reg_set(vop, &win->phy->scl->ext->name, \
> >                           win->base, ~0, v, #name)
> >
> > +#define VOP_AFBC_SET(x, name, v) \
> > +     do { \
> > +             if (vop->data->afbc) \
> > +                     vop_reg_set(vop, &vop->data->afbc->name, \
> > +                             0, ~0, v, #name); \
> > +     } while (0)
> > +
> >  #define VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, name, v) \
> >       do { \
> >               if (win_yuv2yuv && win_yuv2yuv->name.mask) \
> > @@ -123,6 +130,8 @@ struct vop {
> >       struct drm_device *drm_dev;
> >       bool is_enabled;
> >
> > +     struct vop_win *afbc_win;
> > +
> >       struct completion dsp_hold_completion;
> >
> >       /* protected by dev->event_lock */
> > @@ -245,6 +254,30 @@ static bool has_rb_swapped(uint32_t format)
> >       }
> >  }
> >
> > +#define AFBC_FMT_RGB565        0x0
> > +#define AFBC_FMT_U8U8U8U8      0x5
> > +#define AFBC_FMT_U8U8U8        0x4
> > +
> > +static int vop_convert_afbc_format(uint32_t format)
> > +{
> > +     switch (format) {
> > +     case DRM_FORMAT_XRGB8888:
> > +     case DRM_FORMAT_ARGB8888:
> > +     case DRM_FORMAT_XBGR8888:
> > +     case DRM_FORMAT_ABGR8888:
> > +             return AFBC_FMT_U8U8U8U8;
> > +     case DRM_FORMAT_RGB888:
> > +     case DRM_FORMAT_BGR888:
> > +             return AFBC_FMT_U8U8U8;
> > +     case DRM_FORMAT_RGB565:
> > +     case DRM_FORMAT_BGR565:
> > +             return AFBC_FMT_RGB565;
> > +     default:
> > +             DRM_ERROR("unsupported afbc format[%08x]\n", format);
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> >  static enum vop_data_format vop_convert_format(uint32_t format)
> >  {
> >       switch (format) {
> > @@ -587,10 +620,16 @@ static int vop_enable(struct drm_crtc *crtc)
> >
> >               vop_win_disable(vop, win);
> >       }
> > -     spin_unlock(&vop->reg_lock);
> > +
> > +     if (vop->data->afbc) {
> > +             VOP_AFBC_SET(vop, enable, 0);
> > +             vop->afbc_win = NULL;
> > +     }
> >
> >       vop_cfg_done(vop);
> >
> > +     spin_unlock(&vop->reg_lock);
> > +
> >       /*
> >        * At here, vop clock & iommu is enable, R/W vop regs would be safe.
> >        */
> > @@ -719,6 +758,32 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> >               return -EINVAL;
> >       }
> >
> > +     if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> > +             struct vop *vop = to_vop(crtc);
> > +
> > +             if (!vop->data->afbc) {
> > +                     DRM_ERROR("VOP does not support AFBC\n");
> > +                     return -EINVAL;
> > +             }
> > +
> > +             ret = vop_convert_afbc_format(fb->format->format);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             if (state->src.x1 || state->src.y1) {
> > +                     DRM_ERROR("AFBC does not support offset display\n");
> > +                     DRM_ERROR("xpos=%d, ypos=%d, offset=%d\n",
> > +                             state->src.x1, state->src.y1, fb->offsets[0]);
> > +                     return -EINVAL;
> > +             }
> > +
> > +             if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
> > +                     DRM_ERROR("AFBC does not support rotation\n");
> > +                     DRM_ERROR("rotation=%d\n", state->rotation);
> > +                     return -EINVAL;
> > +             }
> > +     }
> > +
> >       return 0;
> >  }
> >
> > @@ -732,6 +797,9 @@ static void vop_plane_atomic_disable(struct drm_plane *plane,
> >       if (!old_state->crtc)
> >               return;
> >
> > +     if (vop->afbc_win == vop_win)
> > +             vop->afbc_win = NULL;
> > +
> >       spin_lock(&vop->reg_lock);
> >
> >       vop_win_disable(vop, win);
> > @@ -774,6 +842,9 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
> >       if (WARN_ON(!vop->is_enabled))
> >               return;
> >
> > +     if (vop->afbc_win == vop_win)
> > +             vop->afbc_win = NULL;
> > +
> >       if (!state->visible) {
> >               vop_plane_atomic_disable(plane, old_state);
> >               return;
> > @@ -808,6 +879,20 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
> >
> >       spin_lock(&vop->reg_lock);
> >
> > +     if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
> > +             int afbc_format = vop_convert_afbc_format(fb->format->format);
> > +
> > +             VOP_AFBC_SET(vop, format, afbc_format | 1 << 4);
> > +             VOP_AFBC_SET(vop, hreg_block_split, 0);
> > +             VOP_AFBC_SET(vop, win_sel, win_index);
> > +             VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> > +             VOP_AFBC_SET(vop, pic_size, act_info);
> > +
> > +             vop->afbc_win = vop_win;
> > +
> > +             pr_info("AFBC on plane %s\n", plane->name);
> > +     }
> > +
> >       VOP_WIN_SET(vop, win, format, format);
> >       VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
> >       VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> > @@ -1163,6 +1248,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> >
> >       spin_lock(&vop->reg_lock);
> >
> > +     VOP_AFBC_SET(vop, enable, vop->afbc_win ? 0x1 : 0);
> >       vop_cfg_done(vop);
> >
> >       spin_unlock(&vop->reg_lock);
> > @@ -1471,7 +1557,8 @@ static int vop_create_crtc(struct vop *vop)
> >                                              0, &vop_plane_funcs,
> >                                              win_data->phy->data_formats,
> >                                              win_data->phy->nformats,
> > -                                            NULL, win_data->type, NULL);
> > +                                            win_data->phy->format_modifiers,
> > +                                            win_data->type, NULL);
> >               if (ret) {
> >                       DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
> >                                     ret);
> > @@ -1511,7 +1598,8 @@ static int vop_create_crtc(struct vop *vop)
> >                                              &vop_plane_funcs,
> >                                              win_data->phy->data_formats,
> >                                              win_data->phy->nformats,
> > -                                            NULL, win_data->type, NULL);
> > +                                            win_data->phy->format_modifiers,
> > +                                            win_data->type, NULL);
> >               if (ret) {
> >                       DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
> >                                     ret);
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > index 2149a889c29d..dc8b12025269 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > @@ -77,6 +77,16 @@ struct vop_misc {
> >       struct vop_reg global_regdone_en;
> >  };
> >
> > +struct vop_afbc {
> > +     struct vop_reg enable;
> > +     struct vop_reg win_sel;
> > +     struct vop_reg format;
> > +     struct vop_reg hreg_block_split;
> > +     struct vop_reg pic_size;
> > +     struct vop_reg hdr_ptr;
> > +     struct vop_reg rstn;
> > +};
> > +
> >  struct vop_intr {
> >       const int *intrs;
> >       uint32_t nintrs;
> > @@ -128,6 +138,7 @@ struct vop_win_phy {
> >       const struct vop_scl_regs *scl;
> >       const uint32_t *data_formats;
> >       uint32_t nformats;
> > +     const uint64_t *format_modifiers;
> >
> >       struct vop_reg enable;
> >       struct vop_reg gate;
> > @@ -169,6 +180,7 @@ struct vop_data {
> >       const struct vop_output *output;
> >       const struct vop_win_yuv2yuv_data *win_yuv2yuv;
> >       const struct vop_win_data *win;
> > +     const struct vop_afbc *afbc;
> >       unsigned int win_size;
> >
> >  #define VOP_FEATURE_OUTPUT_RGB10     BIT(0)
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > index 7b9c74750f6d..e9ff0c43c396 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > @@ -30,6 +30,12 @@
> >  #define VOP_REG_MASK_SYNC(off, _mask, _shift) \
> >               _VOP_REG(off, _mask, _shift, true, false)
> >
> > +static const uint64_t format_modifiers_afbc[] = {
> > +     DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP),
> > +     DRM_FORMAT_MOD_LINEAR,
> > +     DRM_FORMAT_MOD_INVALID
> > +};
> > +
> >  static const uint32_t formats_win_full[] = {
> >       DRM_FORMAT_XRGB8888,
> >       DRM_FORMAT_ARGB8888,
> > @@ -667,6 +673,7 @@ static const struct vop_win_phy rk3368_win01_data = {
> >       .scl = &rk3288_win_full_scl,
> >       .data_formats = formats_win_full,
> >       .nformats = ARRAY_SIZE(formats_win_full),
> > +     .format_modifiers = format_modifiers_afbc,
> >       .enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
> >       .format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
> >       .rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
> > @@ -758,6 +765,16 @@ static const struct vop_data rk3366_vop = {
> >       .win_size = ARRAY_SIZE(rk3368_vop_win_data),
> >  };
> >
> > +static const struct vop_afbc rk3399_afbc = {
> > +     .rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
> > +     .enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
> > +     .win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
> > +     .format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
> > +     .hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
> > +     .hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
> > +     .pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
> > +};
> > +
> >  static const struct vop_output rk3399_output = {
> >       .dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
> >       .rgb_pin_pol = VOP_REG(RK3368_DSP_CTRL1, 0xf, 16),
> > @@ -808,6 +825,7 @@ static const struct vop_data rk3399_vop_big = {
> >       .modeset = &rk3288_modeset,
> >       .output = &rk3399_output,
> >       .misc = &rk3368_misc,
> > +     .afbc = &rk3399_afbc,
> >       .win = rk3368_vop_win_data,
> >       .win_size = ARRAY_SIZE(rk3368_vop_win_data),
> >       .win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 3feeaa3f987a..ba6caf06c824 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -742,6 +742,9 @@ extern "C" {
> >   */
> >  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
> >
> > +#define AFBC_FORMAT_MOD_ROCKCHIP \
> > +     (AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
> > +
> >  /*
> >   * Allwinner tiled modifier
> >   *
> > --
> > 2.17.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch
diff mbox series

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 64ca87cf6d50..5178939a9c29 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -24,6 +24,27 @@  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
 	.dirty	       = drm_atomic_helper_dirtyfb,
 };
 
+static int
+rockchip_verify_afbc_mod(struct drm_device *dev,
+			 const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	if (mode_cmd->modifier[0] &
+	    ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
+		DRM_DEV_ERROR(dev->dev,
+			      "Unsupported format modifier 0x%llx\n",
+			      mode_cmd->modifier[0]);
+		return -EINVAL;
+	}
+
+	if (mode_cmd->width > 2560) {
+		DRM_DEV_ERROR(dev->dev,
+			      "Unsupported width %d\n", mode_cmd->width);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static struct drm_framebuffer *
 rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd,
 		  struct drm_gem_object **obj, unsigned int num_planes)
@@ -32,6 +53,12 @@  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
 	int ret;
 	int i;
 
+	if (mode_cmd->modifier[0]) {
+		ret = rockchip_verify_afbc_mod(dev, mode_cmd);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
 	if (!fb)
 		return ERR_PTR(-ENOMEM);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 21b68eea46cc..50bf214d21da 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -46,6 +46,13 @@ 
 		vop_reg_set(vop, &win->phy->scl->ext->name, \
 			    win->base, ~0, v, #name)
 
+#define VOP_AFBC_SET(x, name, v) \
+	do { \
+		if (vop->data->afbc) \
+			vop_reg_set(vop, &vop->data->afbc->name, \
+				0, ~0, v, #name); \
+	} while (0)
+
 #define VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, name, v) \
 	do { \
 		if (win_yuv2yuv && win_yuv2yuv->name.mask) \
@@ -123,6 +130,8 @@  struct vop {
 	struct drm_device *drm_dev;
 	bool is_enabled;
 
+	struct vop_win *afbc_win;
+
 	struct completion dsp_hold_completion;
 
 	/* protected by dev->event_lock */
@@ -245,6 +254,30 @@  static bool has_rb_swapped(uint32_t format)
 	}
 }
 
+#define AFBC_FMT_RGB565        0x0
+#define AFBC_FMT_U8U8U8U8      0x5
+#define AFBC_FMT_U8U8U8        0x4
+
+static int vop_convert_afbc_format(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		return AFBC_FMT_U8U8U8U8;
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_BGR888:
+		return AFBC_FMT_U8U8U8;
+	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_BGR565:
+		return AFBC_FMT_RGB565;
+	default:
+		DRM_ERROR("unsupported afbc format[%08x]\n", format);
+		return -EINVAL;
+	}
+}
+
 static enum vop_data_format vop_convert_format(uint32_t format)
 {
 	switch (format) {
@@ -587,10 +620,16 @@  static int vop_enable(struct drm_crtc *crtc)
 
 		vop_win_disable(vop, win);
 	}
-	spin_unlock(&vop->reg_lock);
+
+	if (vop->data->afbc) {
+		VOP_AFBC_SET(vop, enable, 0);
+		vop->afbc_win = NULL;
+	}
 
 	vop_cfg_done(vop);
 
+	spin_unlock(&vop->reg_lock);
+
 	/*
 	 * At here, vop clock & iommu is enable, R/W vop regs would be safe.
 	 */
@@ -719,6 +758,32 @@  static int vop_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
+		struct vop *vop = to_vop(crtc);
+
+		if (!vop->data->afbc) {
+			DRM_ERROR("VOP does not support AFBC\n");
+			return -EINVAL;
+		}
+
+		ret = vop_convert_afbc_format(fb->format->format);
+		if (ret < 0)
+			return ret;
+
+		if (state->src.x1 || state->src.y1) {
+			DRM_ERROR("AFBC does not support offset display\n");
+			DRM_ERROR("xpos=%d, ypos=%d, offset=%d\n",
+				state->src.x1, state->src.y1, fb->offsets[0]);
+			return -EINVAL;
+		}
+
+		if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
+			DRM_ERROR("AFBC does not support rotation\n");
+			DRM_ERROR("rotation=%d\n", state->rotation);
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
@@ -732,6 +797,9 @@  static void vop_plane_atomic_disable(struct drm_plane *plane,
 	if (!old_state->crtc)
 		return;
 
+	if (vop->afbc_win == vop_win)
+		vop->afbc_win = NULL;
+
 	spin_lock(&vop->reg_lock);
 
 	vop_win_disable(vop, win);
@@ -774,6 +842,9 @@  static void vop_plane_atomic_update(struct drm_plane *plane,
 	if (WARN_ON(!vop->is_enabled))
 		return;
 
+	if (vop->afbc_win == vop_win)
+		vop->afbc_win = NULL;
+
 	if (!state->visible) {
 		vop_plane_atomic_disable(plane, old_state);
 		return;
@@ -808,6 +879,20 @@  static void vop_plane_atomic_update(struct drm_plane *plane,
 
 	spin_lock(&vop->reg_lock);
 
+	if (fb->modifier & DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP)) {
+		int afbc_format = vop_convert_afbc_format(fb->format->format);
+
+		VOP_AFBC_SET(vop, format, afbc_format | 1 << 4);
+		VOP_AFBC_SET(vop, hreg_block_split, 0);
+		VOP_AFBC_SET(vop, win_sel, win_index);
+		VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
+		VOP_AFBC_SET(vop, pic_size, act_info);
+
+		vop->afbc_win = vop_win;
+
+		pr_info("AFBC on plane %s\n", plane->name);
+	}
+
 	VOP_WIN_SET(vop, win, format, format);
 	VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
 	VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
@@ -1163,6 +1248,7 @@  static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	spin_lock(&vop->reg_lock);
 
+	VOP_AFBC_SET(vop, enable, vop->afbc_win ? 0x1 : 0);
 	vop_cfg_done(vop);
 
 	spin_unlock(&vop->reg_lock);
@@ -1471,7 +1557,8 @@  static int vop_create_crtc(struct vop *vop)
 					       0, &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       NULL, win_data->type, NULL);
+					       win_data->phy->format_modifiers,
+					       win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
 				      ret);
@@ -1511,7 +1598,8 @@  static int vop_create_crtc(struct vop *vop)
 					       &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       NULL, win_data->type, NULL);
+					       win_data->phy->format_modifiers,
+					       win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
 				      ret);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 2149a889c29d..dc8b12025269 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -77,6 +77,16 @@  struct vop_misc {
 	struct vop_reg global_regdone_en;
 };
 
+struct vop_afbc {
+	struct vop_reg enable;
+	struct vop_reg win_sel;
+	struct vop_reg format;
+	struct vop_reg hreg_block_split;
+	struct vop_reg pic_size;
+	struct vop_reg hdr_ptr;
+	struct vop_reg rstn;
+};
+
 struct vop_intr {
 	const int *intrs;
 	uint32_t nintrs;
@@ -128,6 +138,7 @@  struct vop_win_phy {
 	const struct vop_scl_regs *scl;
 	const uint32_t *data_formats;
 	uint32_t nformats;
+	const uint64_t *format_modifiers;
 
 	struct vop_reg enable;
 	struct vop_reg gate;
@@ -169,6 +180,7 @@  struct vop_data {
 	const struct vop_output *output;
 	const struct vop_win_yuv2yuv_data *win_yuv2yuv;
 	const struct vop_win_data *win;
+	const struct vop_afbc *afbc;
 	unsigned int win_size;
 
 #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 7b9c74750f6d..e9ff0c43c396 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -30,6 +30,12 @@ 
 #define VOP_REG_MASK_SYNC(off, _mask, _shift) \
 		_VOP_REG(off, _mask, _shift, true, false)
 
+static const uint64_t format_modifiers_afbc[] = {
+	DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP),
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
 static const uint32_t formats_win_full[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ARGB8888,
@@ -667,6 +673,7 @@  static const struct vop_win_phy rk3368_win01_data = {
 	.scl = &rk3288_win_full_scl,
 	.data_formats = formats_win_full,
 	.nformats = ARRAY_SIZE(formats_win_full),
+	.format_modifiers = format_modifiers_afbc,
 	.enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
 	.rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
@@ -758,6 +765,16 @@  static const struct vop_data rk3366_vop = {
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
 };
 
+static const struct vop_afbc rk3399_afbc = {
+	.rstn = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 3),
+	.enable = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 0),
+	.win_sel = VOP_REG(RK3399_AFBCD0_CTRL, 0x3, 1),
+	.format = VOP_REG(RK3399_AFBCD0_CTRL, 0x1f, 16),
+	.hreg_block_split = VOP_REG(RK3399_AFBCD0_CTRL, 0x1, 21),
+	.hdr_ptr = VOP_REG(RK3399_AFBCD0_HDR_PTR, 0xffffffff, 0),
+	.pic_size = VOP_REG(RK3399_AFBCD0_PIC_SIZE, 0xffffffff, 0),
+};
+
 static const struct vop_output rk3399_output = {
 	.dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
 	.rgb_pin_pol = VOP_REG(RK3368_DSP_CTRL1, 0xf, 16),
@@ -808,6 +825,7 @@  static const struct vop_data rk3399_vop_big = {
 	.modeset = &rk3288_modeset,
 	.output = &rk3399_output,
 	.misc = &rk3368_misc,
+	.afbc = &rk3399_afbc,
 	.win = rk3368_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
 	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3f987a..ba6caf06c824 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -742,6 +742,9 @@  extern "C" {
  */
 #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
 
+#define AFBC_FORMAT_MOD_ROCKCHIP \
+	(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE)
+
 /*
  * Allwinner tiled modifier
  *