Re: [PATCH 7/9] drm/verisilicon: Add drm plane funcs

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

 





Am 02.06.23 um 09:40 schrieb Keith Zhao:
Implement plane functions for the DRM driver.

Signed-off-by: Keith Zhao <keith.zhao@xxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/verisilicon/Makefile   |   3 +-
  drivers/gpu/drm/verisilicon/vs_plane.c | 440 +++++++++++++++++++++++++
  drivers/gpu/drm/verisilicon/vs_plane.h |  74 +++++
  3 files changed, 516 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c
  create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h

diff --git a/drivers/gpu/drm/verisilicon/Makefile b/drivers/gpu/drm/verisilicon/Makefile
index bae5fbab9bbb..d96ad9399fc7 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -3,7 +3,8 @@
  vs_drm-objs := vs_drv.o \
  		vs_crtc.o \
  		vs_fb.o \
-		vs_gem.o
+		vs_gem.o \
+		vs_plane.o
obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
new file mode 100644
index 000000000000..7b0dcef232ae
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_plane.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
+#include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_fb_dma_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_plane_helper.h>
+
+#include <drm/vs_drm.h>
+
+#include "vs_crtc.h"
+#include "vs_fb.h"
+#include "vs_gem.h"
+#include "vs_plane.h"
+#include "vs_type.h"
+
+void vs_plane_destory(struct drm_plane *plane)

'_destroy'

+{
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+
+	drm_plane_cleanup(plane);
+	kfree(vs_plane);
+}

I think we have helpers to do the cleanup.

As I mentioned in another review, you should switch to managed code instead of using your own memory management. If your modesetting pipeline is static, you can use drmm_mode_config_init() and it will release all pipeline structs for you. The related memory can allowed as part of the device instance. If the pipeline is dynamic, you can use drmm_ helpers to allocate conenctors, crtcs, planes, etc. IIRC, the vc4 driver does something like that.

+
+static void vs_plane_reset(struct drm_plane *plane)
+{
+	struct vs_plane_state *state;
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+
+	if (plane->state) {
+		__drm_atomic_helper_plane_destroy_state(plane->state);
+
+		state = to_vs_plane_state(plane->state);
+		kfree(state);
+		plane->state = NULL;
+	}
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return;
+
+	__drm_atomic_helper_plane_reset(plane, &state->base);
+
+	state->degamma = VS_DEGAMMA_DISABLE;
+	state->degamma_changed = false;
+	state->base.zpos = vs_plane->id;
+
+	memset(&state->status, 0, sizeof(state->status));

Should already be 0 from kzalloc().

+}
+
+static void _vs_plane_duplicate_blob(struct vs_plane_state *state,
+				     struct vs_plane_state *ori_state)
+{
+	state->watermark = ori_state->watermark;
+	state->color_mgmt = ori_state->color_mgmt;
+	state->roi = ori_state->roi;
+
+	if (state->watermark)
+		drm_property_blob_get(state->watermark);
+	if (state->color_mgmt)
+		drm_property_blob_get(state->color_mgmt);
+	if (state->roi)
+		drm_property_blob_get(state->roi);
+}
+
+static int
+_vs_plane_set_property_blob_from_id(struct drm_device *dev,
+				    struct drm_property_blob **blob,
+				    u64 blob_id,
+				    size_t expected_size)
+{
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (!new_blob)
+			return -EINVAL;
+
+		if (new_blob->length != expected_size) {
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
+	}
+
+	drm_property_replace_blob(blob, new_blob);
+	drm_property_blob_put(new_blob);
+
+	return 0;
+}
+
+static struct drm_plane_state *
+vs_plane_atomic_duplicate_state(struct drm_plane *plane)
+{
+	struct vs_plane_state *ori_state;
+	struct vs_plane_state *state;
+
+	if (WARN_ON(!plane->state))
+		return NULL;
+
+	ori_state = to_vs_plane_state(plane->state);
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	__drm_atomic_helper_plane_duplicate_state(plane, &state->base);
+
+	state->degamma = ori_state->degamma;
+	state->degamma_changed = ori_state->degamma_changed;
+
+	_vs_plane_duplicate_blob(state, ori_state);
+	memcpy(&state->status, &ori_state->status, sizeof(ori_state->status));
+
+	return &state->base;
+}
+
+static void vs_plane_atomic_destroy_state(struct drm_plane *plane,
+					  struct drm_plane_state *state)
+{
+	struct vs_plane_state *vs_plane_state = to_vs_plane_state(state);
+
+	__drm_atomic_helper_plane_destroy_state(state);
+
+	drm_property_blob_put(vs_plane_state->watermark);
+	drm_property_blob_put(vs_plane_state->color_mgmt);
+	drm_property_blob_put(vs_plane_state->roi);
+	kfree(vs_plane_state);
+}
+
+static int vs_plane_atomic_set_property(struct drm_plane *plane,
+					struct drm_plane_state *state,
+					struct drm_property *property,
+					uint64_t val)
+{
+	struct drm_device *dev = plane->dev;
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+	struct vs_plane_state *vs_plane_state = to_vs_plane_state(state);
+	int ret = 0;
+
+	if (property == vs_plane->degamma_mode) {
+		if (vs_plane_state->degamma != val) {
+			vs_plane_state->degamma = val;
+			vs_plane_state->degamma_changed = true;
+		} else {
+			vs_plane_state->degamma_changed = false;
+		}
+	} else if (property == vs_plane->watermark_prop) {
+		ret = _vs_plane_set_property_blob_from_id(dev,
+							  &vs_plane_state->watermark,
+							  val,
+							  sizeof(struct drm_vs_watermark));
+		return ret;
+	} else if (property == vs_plane->color_mgmt_prop) {
+		ret = _vs_plane_set_property_blob_from_id(dev,
+							  &vs_plane_state->color_mgmt,
+							  val,
+							  sizeof(struct drm_vs_color_mgmt));
+		return ret;
+	} else if (property == vs_plane->roi_prop) {
+		ret = _vs_plane_set_property_blob_from_id(dev,
+							  &vs_plane_state->roi,
+							  val,
+							  sizeof(struct drm_vs_roi));
+		return ret;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int vs_plane_atomic_get_property(struct drm_plane *plane,
+					const struct drm_plane_state *state,
+					struct drm_property *property,
+					uint64_t *val)
+{
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+	const struct vs_plane_state *vs_plane_state =
+		container_of(state, const struct vs_plane_state, base);
+
+	if (property == vs_plane->degamma_mode)
+		*val = vs_plane_state->degamma;
+	else if (property == vs_plane->watermark_prop)
+		*val = (vs_plane_state->watermark) ?
+					vs_plane_state->watermark->base.id : 0;
+	else if (property == vs_plane->color_mgmt_prop)
+		*val = (vs_plane_state->color_mgmt) ?
+					vs_plane_state->color_mgmt->base.id : 0;
+	else if (property == vs_plane->roi_prop)
+		*val = (vs_plane_state->roi) ?
+					vs_plane_state->roi->base.id : 0;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static bool vs_format_mod_supported(struct drm_plane *plane,
+				    u32 format,
+				    u64 modifier)
+{
+	int i;
+
+       /* We always have to allow these modifiers:

Indention seems off.

+	* 1. Core DRM checks for LINEAR support if userspace does not provide modifiers.
+	* 2. Not passing any modifiers is the same as explicitly passing INVALID.
+	*/
+	if (modifier == DRM_FORMAT_MOD_LINEAR)
+		return true;
+
+	/* Check that the modifier is on the list of the plane's supported modifiers. */
+	for (i = 0; i < plane->modifier_count; i++) {
+		if (modifier == plane->modifiers[i])
+			break;
+	}
+
+	if (i == plane->modifier_count)
+		return false;
+
+	return true;
+}
+
+const struct drm_plane_funcs vs_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= vs_plane_destory,
+	.reset			= vs_plane_reset,
+	.atomic_duplicate_state = vs_plane_atomic_duplicate_state,
+	.atomic_destroy_state	= vs_plane_atomic_destroy_state,
+	.atomic_set_property	= vs_plane_atomic_set_property,
+	.atomic_get_property	= vs_plane_atomic_get_property,
+	.format_mod_supported	= vs_format_mod_supported,
+};
+
+static unsigned char vs_get_plane_number(struct drm_framebuffer *fb)
+{
+	const struct drm_format_info *info;
+
+	if (!fb)
+		return 0;
+
+	info = drm_format_info(fb->format->format);
+	if (!info || info->num_planes > MAX_NUM_PLANES)
+		return 0;
+
+	return info->num_planes;
+}
+
+static int vs_plane_atomic_check(struct drm_plane *plane,
+				 struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
+										 plane);
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+	struct drm_framebuffer *fb = new_plane_state->fb;
+	struct drm_crtc *crtc = new_plane_state->crtc;
+	struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+
+	if (!crtc || !fb)
+		return 0;
+
+	//return vs_plane->funcs->check(vs_crtc->dev, vs_plane, new_plane_state);
+	return vs_plane->funcs->check(vs_crtc->dev, plane, state);
+}
+
+static void vs_plane_atomic_update(struct drm_plane *plane,
+				   struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state,
+									   plane);
+	unsigned char i, num_planes;
+	struct drm_framebuffer *fb;
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+	//struct drm_plane_state *state = plane->state;
+	struct vs_crtc *vs_crtc = to_vs_crtc(new_state->crtc);
+	struct vs_plane_state *plane_state = to_vs_plane_state(new_state);
+	//struct drm_format_name_buf *name = &plane_state->status.format_name;
+
+	if (!new_state->fb || !new_state->crtc)
+		return;
+
+	fb = new_state->fb;
+
+	num_planes = vs_get_plane_number(fb);
+
+	for (i = 0; i < num_planes; i++) {
+		struct vs_gem_object *vs_obj;
+
+		vs_obj = vs_fb_get_gem_obj(fb, i);
+		vs_plane->dma_addr[i] = vs_obj->iova + fb->offsets[i];
+	}
+
+	plane_state->status.src = drm_plane_state_src(new_state);
+	plane_state->status.dest = drm_plane_state_dest(new_state);
+
+	vs_plane->funcs->update(vs_crtc->dev, vs_plane, plane, state);
+}
+
+static void vs_plane_atomic_disable(struct drm_plane *plane,
+				    struct drm_atomic_state *state)
+{
+	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
+										   plane);
+	struct vs_plane *vs_plane = to_vs_plane(plane);
+	struct vs_crtc *vs_crtc = to_vs_crtc(old_state->crtc);
+
+	vs_plane->funcs->disable(vs_crtc->dev, vs_plane, old_state);
+}
+
+const struct drm_plane_helper_funcs vs_plane_helper_funcs = {
+	.atomic_check	= vs_plane_atomic_check,
+	.atomic_update	= vs_plane_atomic_update,
+	.atomic_disable = vs_plane_atomic_disable,
+};
+
+static const struct drm_prop_enum_list vs_degamma_mode_enum_list[] = {
+	{ VS_DEGAMMA_DISABLE,	"disabled" },
+	{ VS_DEGAMMA_BT709, "preset degamma for BT709" },
+	{ VS_DEGAMMA_BT2020,	"preset degamma for BT2020" },
+};
+
+struct vs_plane *vs_plane_create(struct drm_device *drm_dev,
+				 struct vs_plane_info *info,
+				 unsigned int layer_num,
+				 unsigned int possible_crtcs)
+{
+	struct vs_plane *plane;
+	int ret;
+
+	if (!info)
+		return NULL;
+
+	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return NULL;
+
+	ret = drm_universal_plane_init(drm_dev, &plane->base, possible_crtcs,
+				       &vs_plane_funcs, info->formats,
+				       info->num_formats, info->modifiers, info->type,
+				       info->name ? info->name : NULL);
+	if (ret)
+		goto err_free_plane;
+
+	drm_plane_helper_add(&plane->base, &vs_plane_helper_funcs);
+
+	/* Set up the plane properties */
+	if (info->degamma_size) {
+		plane->degamma_mode =
+		drm_property_create_enum(drm_dev, 0,
+					 "DEGAMMA_MODE",
+					 vs_degamma_mode_enum_list,
+					 ARRAY_SIZE(vs_degamma_mode_enum_list));
+
+		if (!plane->degamma_mode)
+			goto error_cleanup_plane;
+
+		drm_object_attach_property(&plane->base.base,
+					   plane->degamma_mode,
+					   VS_DEGAMMA_DISABLE);
+	}
+
+	if (info->rotation) {
+		ret = drm_plane_create_rotation_property(&plane->base,
+							 DRM_MODE_ROTATE_0,
+							 info->rotation);
+		if (ret)
+			goto error_cleanup_plane;
+	}
+
+	if (info->blend_mode) {
+		ret = drm_plane_create_blend_mode_property(&plane->base,
+							   info->blend_mode);
+		if (ret)
+			goto error_cleanup_plane;
+		ret = drm_plane_create_alpha_property(&plane->base);
+		if (ret)
+			goto error_cleanup_plane;
+	}
+
+	if (info->color_encoding) {
+		ret = drm_plane_create_color_properties(&plane->base,
+							info->color_encoding,
+							BIT(DRM_COLOR_YCBCR_LIMITED_RANGE),
+							DRM_COLOR_YCBCR_BT709,
+							DRM_COLOR_YCBCR_LIMITED_RANGE);
+		if (ret)
+			goto error_cleanup_plane;
+	}
+
+	if (info->zpos != 255) {
+		ret = drm_plane_create_zpos_property(&plane->base, info->zpos, 0,
+						     layer_num - 1);
+		if (ret)
+			goto error_cleanup_plane;
+	} else {
+		ret = drm_plane_create_zpos_immutable_property(&plane->base,
+							       info->zpos);
+		if (ret)
+			goto error_cleanup_plane;
+	}
+
+	if (info->watermark) {
+		plane->watermark_prop = drm_property_create(drm_dev, DRM_MODE_PROP_BLOB,
+							    "WATERMARK", 0);
+		if (!plane->watermark_prop)
+			goto error_cleanup_plane;
+
+		drm_object_attach_property(&plane->base.base, plane->watermark_prop, 0);
+	}
+
+	if (info->color_mgmt) {
+		plane->color_mgmt_prop = drm_property_create(drm_dev, DRM_MODE_PROP_BLOB,
+							     "COLOR_CONFIG", 0);
+		if (!plane->color_mgmt_prop)
+			goto error_cleanup_plane;
+
+		drm_object_attach_property(&plane->base.base, plane->color_mgmt_prop, 0);
+	}
+
+	if (info->roi) {
+		plane->roi_prop = drm_property_create(drm_dev, DRM_MODE_PROP_BLOB,
+						      "ROI", 0);
+		if (!plane->roi_prop)
+			goto error_cleanup_plane;
+
+		drm_object_attach_property(&plane->base.base, plane->roi_prop, 0);
+	}
+
+	return plane;
+
+error_cleanup_plane:
+	drm_plane_cleanup(&plane->base);
+err_free_plane:
+	kfree(plane);
+	return NULL;
+}
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.h b/drivers/gpu/drm/verisilicon/vs_plane.h
new file mode 100644
index 000000000000..76ef3c3de045
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_plane.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+
+#ifndef __VS_PLANE_H__
+#define __VS_PLANE_H__
+
+#include <drm/drm_fourcc.h>
+#include <drm/drm_plane_helper.h>
+
+#include "vs_fb.h"
+#include "vs_type.h"
+
+struct vs_plane;
+
+struct vs_plane_funcs {
+	void (*update)(struct device *dev, struct vs_plane *plane, struct drm_plane *drm_plane,
+		       struct drm_atomic_state *state);
+	void (*disable)(struct device *dev, struct vs_plane *plane,
+			struct drm_plane_state *old_state);
+	int (*check)(struct device *dev, struct drm_plane *plane,
+		     struct drm_atomic_state *state);
+};

Your own internal API. Exactly the same problems as in the CRTC case.

+
+struct vs_plane_status {
+	u32 tile_mode;
+	struct drm_rect src;
+	struct drm_rect dest;
+};
+
+struct vs_plane_state {
+	struct drm_plane_state base;
+	struct vs_plane_status status; /* for debugfs */
+
+	struct drm_property_blob *watermark;
+	struct drm_property_blob *color_mgmt;
+	struct drm_property_blob *roi;
+
+	u32 degamma;
+	bool degamma_changed;
+};
+
+struct vs_plane {
+	struct drm_plane base;
+	u8 id;

I'm not sure what this is for. It's used for zpos. If that's the purpose, I'd rather call it default_zpos.

Best regards
Thomas

+	dma_addr_t dma_addr[MAX_NUM_PLANES];
+
+	struct drm_property *degamma_mode;
+	struct drm_property *watermark_prop;
+	struct drm_property *color_mgmt_prop;
+	struct drm_property *roi_prop;
+
+	const struct vs_plane_funcs *funcs;
+};
+
+void vs_plane_destory(struct drm_plane *plane);
+
+struct vs_plane *vs_plane_create(struct drm_device *drm_dev,
+				 struct vs_plane_info *info,
+				 unsigned int layer_num,
+				 unsigned int possible_crtcs);
+
+static inline struct vs_plane *to_vs_plane(struct drm_plane *plane)
+{
+	return container_of(plane, struct vs_plane, base);
+}
+
+static inline struct vs_plane_state *
+to_vs_plane_state(struct drm_plane_state *state)
+{
+	return container_of(state, struct vs_plane_state, base);
+}
+#endif /* __VS_PLANE_H__ */

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[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