Re: [PATCH] drm/rockchip: Add AFBC support
On Wed, Sep 25, 2019 at 11:35 AM Ayan Halder wrote:
>
> On Mon, Sep 23, 2019 at 02:20:13PM +0200, Andrzej Pietrasiewicz wrote:
> > From: Ezequiel Garcia
> >
> > 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
> > [locking improvements]
> > Signed-off-by: Tomeu Vizoso
> > [squashing the above, commit message and Rockchip AFBC modifier]
> > Signed-off-by: Andrzej Pietrasiewicz
> > ---
> > 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, >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, >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_RGB5650x0
> > +#define AFBC_FMT_U8U8U8U8 0x5
> > +#define AFBC_FMT_U8U8U80x4
> > +
> > +static int vop_convert_afbc_format(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_XRGB:
> > + case DRM_FORMAT_ARGB:
> > + case DRM_FORMAT_XBGR:
> > + case DRM_FORMAT_ABGR:
> > + 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;
> >
Re: [PATCH] drm/rockchip: Add AFBC support
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.hal...@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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: Add AFBC support
Hi Andrzej,
Thanks for the patch, it's nice to see another AFBC implementation
coming in.
For future versions, could you please Cc ayan.hal...@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
>
> 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
> [locking improvements]
> Signed-off-by: Tomeu Vizoso
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz
> ---
> 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, >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, >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_RGB5650x0
> +#define AFBC_FMT_U8U8U8U8 0x5
> +#define AFBC_FMT_U8U8U80x4
> +
> +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_XRGB:
XRGB: Not preferred, as encoding the X channel is a waste of bits
> + case DRM_FORMAT_ARGB:
ARGB: Not preferred as the channel order prevents YTR
> + case DRM_FORMAT_XBGR:
XBGR: Not preferred, as encoding the X channel is a waste of bits
> + case DRM_FORMAT_ABGR:
> + return AFBC_FMT_U8U8U8U8;
> +
Re: [PATCH] drm/rockchip: Add AFBC support
On Mon, Sep 23, 2019 at 02:20:13PM +0200, Andrzej Pietrasiewicz wrote:
> From: Ezequiel Garcia
>
> 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
> [locking improvements]
> Signed-off-by: Tomeu Vizoso
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz
> ---
> 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, >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, >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_RGB5650x0
> +#define AFBC_FMT_U8U8U8U8 0x5
> +#define AFBC_FMT_U8U8U80x4
> +
> +static int vop_convert_afbc_format(uint32_t format)
> +{
> + switch (format) {
> + case DRM_FORMAT_XRGB:
> + case DRM_FORMAT_ARGB:
> + case DRM_FORMAT_XBGR:
> + case DRM_FORMAT_ABGR:
> + 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(>reg_lock);
> +
> + if (vop->data->afbc) {
> + VOP_AFBC_SET(vop, enable, 0);
> + vop->afbc_win = NULL;
> + }
>
> vop_cfg_done(vop);
>
Re: [PATCH] drm/rockchip: Add AFBC support
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
> >>
> >>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
> >>[locking improvements]
> >>Signed-off-by: Tomeu Vizoso
> >>[squashing the above, commit message and Rockchip AFBC modifier]
> >>Signed-off-by: Andrzej Pietrasiewicz
> >>---
> >> 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
Re: [PATCH] drm/rockchip: Add AFBC support
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
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
[locking improvements]
Signed-off-by: Tomeu Vizoso
[squashing the above, commit message and Rockchip AFBC modifier]
Signed-off-by: Andrzej Pietrasiewicz
---
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
*
Re: [PATCH] drm/rockchip: Add AFBC support
On 23/09/2019 14:20, Andrzej Pietrasiewicz wrote:
> From: Ezequiel Garcia
>
> 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
> [locking improvements]
> Signed-off-by: Tomeu Vizoso
> [squashing the above, commit message and Rockchip AFBC modifier]
> Signed-off-by: Andrzej Pietrasiewicz
> ---
> 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
> *
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/rockchip: Add AFBC support
From: Ezequiel Garcia
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
[locking improvements]
Signed-off-by: Tomeu Vizoso
[squashing the above, commit message and Rockchip AFBC modifier]
Signed-off-by: Andrzej Pietrasiewicz
---
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, >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, >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_RGB5650x0
+#define AFBC_FMT_U8U8U8U8 0x5
+#define AFBC_FMT_U8U8U80x4
+
+static int vop_convert_afbc_format(uint32_t format)
+{
+ switch (format) {
+ case DRM_FORMAT_XRGB:
+ case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_XBGR:
+ case DRM_FORMAT_ABGR:
+ 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(>reg_lock);
+
+ if (vop->data->afbc) {
+ VOP_AFBC_SET(vop, enable, 0);
+ vop->afbc_win = NULL;
+ }
vop_cfg_done(vop);
+ spin_unlock(>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) {
+
