Hi Daniel,
-----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, January 09, 2018 1:38 AM To: Hyun Kwon hyunk@xilinx.com Cc: dri-devel@lists.freedesktop.org; devicetree@vger.kernel.org; Michal Simek michal.simek@xilinx.com Subject: Re: [PATCH 02/10] drm: xlnx: Add xlnx crtc of Xilinx DRM KMS
On Thu, Jan 04, 2018 at 06:05:51PM -0800, Hyun Kwon wrote:
xlnx_crtc is a part of Xilinx DRM KMS and a layer that provides some interface between the Xilinx DRM KMS and crtc drivers.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
Personal style, but I don't see much value in these small helpers. Splitting them from the main driver at least makes reading the patches a bit harder. But no strong opinion, just a bikeshed, feel free to ignore :-)
I also don't, but some reviewers prefer this way. I'll leave it this way for now. :-)
Thanks, -hyun
-Daniel
drivers/gpu/drm/xlnx/xlnx_crtc.c | 194
+++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/xlnx/xlnx_crtc.h | 70 ++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.c create mode 100644 drivers/gpu/drm/xlnx/xlnx_crtc.h
diff --git a/drivers/gpu/drm/xlnx/xlnx_crtc.c
b/drivers/gpu/drm/xlnx/xlnx_crtc.c
new file mode 100644 index 0000000..57ee939 --- /dev/null +++ b/drivers/gpu/drm/xlnx/xlnx_crtc.c @@ -0,0 +1,194 @@ +/*
- Xilinx DRM crtc driver
- Copyright (C) 2017 - 2018 Xilinx, Inc.
- Author: Hyun Woo Kwon hyun.kwon@xilinx.com
- SPDX-License-Identifier: GPL-2.0
- */
+#include <drm/drmP.h>
+#include <linux/list.h>
+#include "xlnx_crtc.h"
+/*
- Overview
- The Xilinx CRTC layer is to enable the custom interface to CRTC drivers.
- The interface is used by Xilinx DRM driver where it needs CRTC
- functionailty. CRTC drivers should attach the desired callbacks
- to struct xlnx_crtc and register the xlnx_crtc with correcsponding
- drm_device. It's highly recommended CRTC drivers register all
callbacks
- even though many of them are optional.
- The CRTC helper simply walks through the registered CRTC device,
- and call the callbacks.
- */
+/**
- struct xlnx_crtc_helper - Xilinx CRTC helper
- @xlnx_crtcs: list of Xilinx CRTC devices
- @lock: lock to protect @xlnx_crtcs
- @drm: back pointer to DRM core
- */
+struct xlnx_crtc_helper {
- struct list_head xlnx_crtcs;
- struct mutex lock; /* lock for @xlnx_crtcs */
- struct drm_device *drm;
+};
+#define XLNX_CRTC_MAX_HEIGHT_WIDTH UINT_MAX
+int xlnx_crtc_helper_enable_vblank(struct xlnx_crtc_helper *helper,
unsigned int crtc_id)+{
- struct xlnx_crtc *crtc;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list)
if (drm_crtc_index(&crtc->crtc) == crtc_id)if (crtc->enable_vblank)return crtc->enable_vblank(crtc);- return -ENODEV;
+}
+void xlnx_crtc_helper_disable_vblank(struct xlnx_crtc_helper *helper,
unsigned int crtc_id)+{
- struct xlnx_crtc *crtc;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (drm_crtc_index(&crtc->crtc) == crtc_id) {if (crtc->disable_vblank)crtc->disable_vblank(crtc);return;}- }
+}
+unsigned int xlnx_crtc_helper_get_align(struct xlnx_crtc_helper *helper) +{
- struct xlnx_crtc *crtc;
- unsigned int align = 1, tmp;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (crtc->get_align) {tmp = crtc->get_align(crtc);align = ALIGN(align, tmp);}- }
- return align;
+}
+u64 xlnx_crtc_helper_get_dma_mask(struct xlnx_crtc_helper *helper) +{
- struct xlnx_crtc *crtc;
- u64 mask = DMA_BIT_MASK(sizeof(dma_addr_t) * 8), tmp;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (crtc->get_dma_mask) {tmp = crtc->get_dma_mask(crtc);mask = min(mask, tmp);}- }
- return mask;
+}
+int xlnx_crtc_helper_get_max_width(struct xlnx_crtc_helper *helper) +{
- struct xlnx_crtc *crtc;
- unsigned int width = XLNX_CRTC_MAX_HEIGHT_WIDTH, tmp;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (crtc->get_max_width) {tmp = crtc->get_max_width(crtc);width = min(width, tmp);}- }
- return width;
+}
+int xlnx_crtc_helper_get_max_height(struct xlnx_crtc_helper *helper) +{
- struct xlnx_crtc *crtc;
- unsigned int height = XLNX_CRTC_MAX_HEIGHT_WIDTH, tmp;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (crtc->get_max_height) {tmp = crtc->get_max_height(crtc);height = min(height, tmp);}- }
- return height;
+}
+uint32_t xlnx_crtc_helper_get_format(struct xlnx_crtc_helper *helper) +{
- struct xlnx_crtc *crtc;
- u32 format = 0, tmp;
- list_for_each_entry(crtc, &helper->xlnx_crtcs, list) {
if (crtc->get_format) {tmp = crtc->get_format(crtc);if (format && format != tmp)return 0;format = tmp;}- }
- return format;
+}
+struct xlnx_crtc_helper *xlnx_crtc_helper_init(struct drm_device *drm) +{
- struct xlnx_crtc_helper *helper;
- helper = devm_kzalloc(drm->dev, sizeof(*helper), GFP_KERNEL);
- if (!helper)
return ERR_PTR(-ENOMEM);- INIT_LIST_HEAD(&helper->xlnx_crtcs);
- mutex_init(&helper->lock);
- helper->drm = drm;
- return helper;
+}
+void xlnx_crtc_helper_fini(struct drm_device *drm,
struct xlnx_crtc_helper *helper)+{
- if (WARN_ON(helper->drm != drm))
return;- if (WARN_ON(!list_empty(&helper->xlnx_crtcs)))
return;- mutex_destroy(&helper->lock);
- devm_kfree(drm->dev, helper);
+}
+void xlnx_crtc_register(struct drm_device *drm, struct xlnx_crtc *crtc) +{
- struct xlnx_crtc_helper *helper = xlnx_get_crtc_helper(drm);
- mutex_lock(&helper->lock);
- list_add_tail(&crtc->list, &helper->xlnx_crtcs);
- mutex_unlock(&helper->lock);
+} +EXPORT_SYMBOL_GPL(xlnx_crtc_register);
+void xlnx_crtc_unregister(struct drm_device *drm, struct xlnx_crtc *crtc) +{
- struct xlnx_crtc_helper *helper = xlnx_get_crtc_helper(drm);
- mutex_lock(&helper->lock);
- list_del(&crtc->list);
- mutex_unlock(&helper->lock);
+} +EXPORT_SYMBOL_GPL(xlnx_crtc_unregister); diff --git a/drivers/gpu/drm/xlnx/xlnx_crtc.h
b/drivers/gpu/drm/xlnx/xlnx_crtc.h
new file mode 100644 index 0000000..db7404e --- /dev/null +++ b/drivers/gpu/drm/xlnx/xlnx_crtc.h @@ -0,0 +1,70 @@ +/*
- Xilinx DRM crtc header
- Copyright (C) 2017 - 2018 Xilinx, Inc.
- Author: Hyun Woo Kwon hyun.kwon@xilinx.com
- SPDX-License-Identifier: GPL-2.0
- */
+#ifndef _XLNX_CRTC_H_ +#define _XLNX_CRTC_H_
+/**
- struct xlnx_crtc - Xilinx CRTC device
- @crtc: DRM CRTC device
- @list: list node for Xilinx CRTC device list
- @enable_vblank: Enable vblank
- @disable_vblank: Disable vblank
- @get_align: Get the alignment requirement of CRTC device
- @get_dma_mask: Get the dma mask of CRTC device
- @get_max_width: Get the maximum supported width
- @get_max_height: Get the maximum supported height
- @get_format: Get the current format of CRTC device
- */
+struct xlnx_crtc {
- struct drm_crtc crtc;
- struct list_head list;
- int (*enable_vblank)(struct xlnx_crtc *crtc);
- void (*disable_vblank)(struct xlnx_crtc *crtc);
- unsigned int (*get_align)(struct xlnx_crtc *crtc);
- u64 (*get_dma_mask)(struct xlnx_crtc *crtc);
- int (*get_max_width)(struct xlnx_crtc *crtc);
- int (*get_max_height)(struct xlnx_crtc *crtc);
- uint32_t (*get_format)(struct xlnx_crtc *crtc);
+};
+/*
- Helper functions: used within Xlnx DRM
- */
+struct xlnx_crtc_helper;
+int xlnx_crtc_helper_enable_vblank(struct xlnx_crtc_helper *helper,
unsigned int crtc_id);+void xlnx_crtc_helper_disable_vblank(struct xlnx_crtc_helper *helper,
unsigned int crtc_id);+unsigned int xlnx_crtc_helper_get_align(struct xlnx_crtc_helper *helper); +u64 xlnx_crtc_helper_get_dma_mask(struct xlnx_crtc_helper *helper); +int xlnx_crtc_helper_get_max_width(struct xlnx_crtc_helper *helper); +int xlnx_crtc_helper_get_max_height(struct xlnx_crtc_helper *helper); +uint32_t xlnx_crtc_helper_get_format(struct xlnx_crtc_helper *helper);
+struct xlnx_crtc_helper *xlnx_crtc_helper_init(struct drm_device *drm); +void xlnx_crtc_helper_fini(struct drm_device *drm,
struct xlnx_crtc_helper *helper);+/*
- CRTC registration: used by other sub-driver modules
- */
+static inline struct xlnx_crtc *to_xlnx_crtc(struct drm_crtc *crtc) +{
- return container_of(crtc, struct xlnx_crtc, crtc);
+}
+void xlnx_crtc_register(struct drm_device *drm, struct xlnx_crtc *crtc); +void xlnx_crtc_unregister(struct drm_device *drm, struct xlnx_crtc
*crtc);
+#endif /* _XLNX_CRTC_H_ */
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch