Hello Andrzej,
Thanks a lot for the patch.
Reviewed-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
On Thu, 2019-11-21 at 18:22 +0100, Andrzej Pietrasiewicz wrote:
> This patch adds support for afbc handling. afbc is a compressed format
> which reduces the necessary memory bandwidth.
>
> Co-developed-by: Mark Yao <mark.yao@xxxxxxxxxxxxxx>
> Signed-off-by: Mark Yao <mark.yao@xxxxxxxxxxxxxx>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 29 ++++
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 142 +++++++++++++++++++-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 12 ++
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 84 +++++++++++-
> 4 files changed, 263 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index ca01234c037c..7eaa3fdc03b2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -8,6 +8,7 @@
>
> #include <drm/drm.h>
> #include <drm/drm_atomic.h>
> +#include <drm/drm_afbc.h>
> #include <drm/drm_damage_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_fourcc.h>
> @@ -18,6 +19,8 @@
> #include "rockchip_drm_fb.h"
> #include "rockchip_drm_gem.h"
>
> +#define ROCKCHIP_MAX_AFBC_WIDTH 2560
> +
> static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> @@ -32,6 +35,32 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
> int ret;
> int i;
>
> + if (drm_is_afbc(mode_cmd->modifier[0])) {
> + struct drm_afbc afbc;
> +
> + drm_afbc_get_parameters(mode_cmd, &afbc);
> +
> + if (afbc.offset) {
> + DRM_WARN("AFBC plane offset must be zero!\n");
> +
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (afbc.tile_w != 16 || afbc.tile_h != 16) {
> + DRM_WARN("Unsupported afbc tile w/h [%d/%d]\n",
> + afbc.tile_w, afbc.tile_h);
> +
I think it's important to stick to using always "AFBC" or
always ", i.e. to be consisten in user messages.
Makes grepping easier.
[..]
> @@ -846,6 +960,23 @@ 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_BLOCK_SIZE_16x16 |
> + AFBC_FORMAT_MOD_SPARSE)) {
You check this modifier condition a few times, how about
having a helper for it?
> + int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> + VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
> + VOP_AFBC_SET(vop, hreg_block_split, 0);
> + VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
> + VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> + VOP_AFBC_SET(vop, pic_size, act_info);
> +
> + /*
> + * The window being udated becomes the AFBC window
> + */
> + vop->afbc_win = vop_win;
> + }
> +
> 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);
> @@ -1001,6 +1132,7 @@ static const struct drm_plane_funcs vop_plane_funcs = {
> .reset = drm_atomic_helper_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> + .format_mod_supported = rockchip_mod_supported,
> };
>
> static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
> @@ -1340,6 +1472,10 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>
> spin_lock(&vop->reg_lock);
>
> + /*
> + * Enable AFBC if there is some AFBC window, disable otherwise
> + */
Nitpick: no need for multi-line style comments, if the comment
is a single line. Also, you might want to end the comment with a stop.
Thanks!
Eze
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
