Hi Andrzej,
On Fri, 11 Oct 2019 at 12:18, Andrzej Pietrasiewicz
<andrzej.p@xxxxxxxxxxxxx> wrote:
> @@ -32,6 +35,46 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
> int ret;
> int i;
>
> + if (mode_cmd->modifier[0]) {
> + const struct drm_format_info *info;
> + int bpp;
> +
> + if (mode_cmd->modifier[0] &
Use != here, not &~.
> + 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);
This should not be reachable, because you shouldn't be able to create
a framebuffer with an unsupported format/modifier combination.
> +static bool rockchip_mod_supported(struct drm_plane *plane,
> + u32 format, u64 modifier)
> +{
> + const struct drm_format_info *info;
> +
> + if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> + return false;
> +
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + return true;
> +
> + if (modifier & ~DRM_FORMAT_MOD_ARM_AFBC(
Again use != here.
> + AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> + AFBC_FORMAT_MOD_SPARSE
> + )
> + ) {
> + DRM_DEBUG_KMS("Unsupported format modifer 0x%llx\n", modifier);
> +
> + return false;
> + }
> +
> + info = drm_format_info(format);
> + if (info->num_planes != 1) {
> + DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
> + return false;
> + }
This is where you should reject unsupported format + AFBC
combinations. Doing it here prevents it from ever being advertised to
userspace in the first place.
> @@ -808,6 +919,24 @@ 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)) {
> + int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> + VOP_AFBC_SET(vop, format, afbc_format | 1 << 4);
I assume the (1 << 4) programs the 16x16 block size. Can we support
other block sizes?
> + VOP_AFBC_SET(vop, hreg_block_split, 0);
Does this mean we can also support AFBC_FORMAT_MOD_SPLIT?
> + 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;
> +
> + pr_info("AFBC on plane %s\n", plane->name);
Please do not use pr_info() here. Userspace should not be able to
trigger logging, apart from DRM_DEBUG.
> +static const uint64_t format_modifiers_win_full[] = {
> + DRM_FORMAT_MOD_NONE,
*DRM_FORMAT_MOD_LINEAR
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
