Implement live video input format setting for ZynqMP DPSUB.
ZynqMP DPSUB can operate in 2 modes: DMA-based and live.
In the live mode, DPSUB receives a live video signal from FPGA-based CRTC.
DPSUB acts as a DRM encoder bridge in such a scenario. To properly tune
into the incoming video signal, DPSUB should be programmed with the proper
media bus format. This patch series addresses this task.
Patch 1/9: Set the DPSUB layer mode of operation prior to enabling the
layer. Allows to use layer operational mode before its enablement.
Patch 2/9: Update some IP register defines.
Patch 3/9: Factor out some code into a helper function.
Patch 4/9: Announce supported input media bus formats via
drm_bridge_funcs.atomic_get_input_bus_fmts callback.
Patch 5/9: Minimize usage of a global flag. Minor improvement.
Patch 6/9: Program DPSUB live video input format based on selected bus
config in the new atomic bridge state.
Patch 7/9: New optional CRTC atomic helper proposal that will allow CRTC
to participate in DRM bridge chain format negotiation and impose format
restrictions. Incorporate this callback into the DRM bridge format
negotiation process.
Patch 8/9: DT bindings documentation for Video Timing Controller and Test
Pattern Generator IPs.
Patch 9/9: Reference FPGA CRTC driver based on AMD/Xilinx Test Pattern
Generator (TPG) IP. Add driver for the AMD/Xilinx Video Timing Controller
(VTC), which supplements TPG.
To: Laurent Pinchart <[email protected]>
To: Maarten Lankhorst <[email protected]>
To: Maxime Ripard <[email protected]>
To: Thomas Zimmermann <[email protected]>
To: David Airlie <[email protected]>
To: Daniel Vetter <[email protected]>
To: Michal Simek <[email protected]>
To: Andrzej Hajda <[email protected]>
To: Neil Armstrong <[email protected]>
To: Robert Foss <[email protected]>
To: Jonas Karlman <[email protected]>
To: Jernej Skrabec <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anatoliy Klymenko <[email protected]>
Changes in v3:
- Add connected live layer helper
- Include reference DRM format in zynqmp_disp_format for live layerss.
- Add default bus format list for non-live case.
- Explain removal of redundant checks in the commit message.
- Minor fixes and improvements from review comments.
Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Factor out register defines update into separate patch.
- Add some improvements minimizing ithe usage of a global flag.
- Reuse existing format setting API instead of introducing new versions.
- Add warning around NULL check on new bridge state within atomic enable
callback.
- Add drm_helper_crtc_select_output_bus_format() that wraps
drm_crtc_helper_funcs.select_output_bus_format().
- Update API comments per review recommendations.
- Address some minor review comments.
- Add reference CRTC driver that demonstrates the usage of the proposed
drm_crtc_helper_funcs.select_output_bus_format() API.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Anatoliy Klymenko (9):
drm: xlnx: zynqmp_dpsub: Set layer mode during creation
drm: xlnx: zynqmp_dpsub: Update live format defines
drm: xlnx: zynqmp_dpsub: Add connected live layer helper
drm: xlnx: zynqmp_dpsub: Anounce supported input formats
drm: xlnx: zynqmp_dpsub: Minimize usage of global flag
drm: xlnx: zynqmp_dpsub: Set input live format
drm/atomic-helper: Add select_output_bus_format callback
dt-bindings: xlnx: Add VTC and TPG bindings
drm: xlnx: Intoduce TPG CRTC driver
.../bindings/display/xlnx/xlnx,v-tpg.yaml | 87 +++
.../devicetree/bindings/display/xlnx/xlnx,vtc.yaml | 65 ++
drivers/gpu/drm/drm_bridge.c | 14 +-
drivers/gpu/drm/drm_crtc_helper.c | 36 +
drivers/gpu/drm/xlnx/Kconfig | 21 +
drivers/gpu/drm/xlnx/Makefile | 4 +
drivers/gpu/drm/xlnx/xlnx_tpg.c | 846 +++++++++++++++++++++
drivers/gpu/drm/xlnx/xlnx_vtc.c | 452 +++++++++++
drivers/gpu/drm/xlnx/xlnx_vtc.h | 101 +++
drivers/gpu/drm/xlnx/xlnx_vtc_list.c | 160 ++++
drivers/gpu/drm/xlnx/zynqmp_disp.c | 170 ++++-
drivers/gpu/drm/xlnx/zynqmp_disp.h | 19 +-
drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 +-
drivers/gpu/drm/xlnx/zynqmp_dp.c | 81 +-
drivers/gpu/drm/xlnx/zynqmp_kms.c | 6 +-
include/drm/drm_crtc_helper.h | 5 +
include/drm/drm_modeset_helper_vtables.h | 30 +
include/dt-bindings/media/media-bus-format.h | 177 +++++
18 files changed, 2198 insertions(+), 84 deletions(-)
---
base-commit: bfa4437fd3938ae2e186e7664b2db65bb8775670
change-id: 20240226-dp-live-fmt-6415773b5a68
Best regards,
--
Anatoliy Klymenko <[email protected]>
Set layer mode of operation (live or dma-based) during layer creation.
Each DPSUB layer mode of operation is defined by corresponding DT node port
connection, so it is possible to assign it during layer object creation.
Previously it was set in layer enable functions, although it is too late
as setting layer format depends on layer mode, and should be done before
given layer enabled.
Signed-off-by: Anatoliy Klymenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 ++++++++++++++++----
drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +------------
drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 8a39b3accce5..e6d26ef60e89 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -64,6 +64,16 @@
#define ZYNQMP_DISP_MAX_NUM_SUB_PLANES 3
+/**
+ * enum zynqmp_dpsub_layer_mode - Layer mode
+ * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
+ * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
+ */
+enum zynqmp_dpsub_layer_mode {
+ ZYNQMP_DPSUB_LAYER_NONLIVE,
+ ZYNQMP_DPSUB_LAYER_LIVE,
+};
+
/**
* struct zynqmp_disp_format - Display subsystem format information
* @drm_fmt: DRM format (4CC)
@@ -902,15 +912,12 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
/**
* zynqmp_disp_layer_enable - Enable a layer
* @layer: The layer
- * @mode: Operating mode of layer
*
* Enable the @layer in the audio/video buffer manager and the blender. DMA
* channels are started separately by zynqmp_disp_layer_update().
*/
-void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
- enum zynqmp_dpsub_layer_mode mode)
+void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
{
- layer->mode = mode;
zynqmp_disp_avbuf_enable_video(layer->disp, layer);
zynqmp_disp_blend_layer_enable(layer->disp, layer);
}
@@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
layer->id = i;
layer->disp = disp;
layer->info = &layer_info[i];
+ /* For now assume dpsub works in either live or non-live mode for both layers.
+ * Hybrid mode is not supported yet.
+ */
+ layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
+ : ZYNQMP_DPSUB_LAYER_LIVE;
ret = zynqmp_disp_layer_request_dma(disp, layer);
if (ret)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
index 123cffac08be..9b8b202224d9 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
@@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
ZYNQMP_DPSUB_LAYER_GFX,
};
-/**
- * enum zynqmp_dpsub_layer_mode - Layer mode
- * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
- * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
- */
-enum zynqmp_dpsub_layer_mode {
- ZYNQMP_DPSUB_LAYER_NONLIVE,
- ZYNQMP_DPSUB_LAYER_LIVE,
-};
-
void zynqmp_disp_enable(struct zynqmp_disp *disp);
void zynqmp_disp_disable(struct zynqmp_disp *disp);
int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
@@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
unsigned int *num_formats);
-void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
- enum zynqmp_dpsub_layer_mode mode);
+void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
const struct drm_format_info *info);
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 1846c4971fd8..04b6bcac3b07 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1295,7 +1295,7 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
/* TODO: Make the format configurable. */
info = drm_format_info(DRM_FORMAT_YUV422);
zynqmp_disp_layer_set_format(layer, info);
- zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_LIVE);
+ zynqmp_disp_layer_enable(layer);
if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index db3bb4afbfc4..43bf416b33d5 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -122,7 +122,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
/* Enable or re-enable the plane if the format has changed. */
if (format_changed)
- zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_NONLIVE);
+ zynqmp_disp_layer_enable(layer);
}
static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
--
2.25.1
Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG register
layout.
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
index f92a006d5070..fa3935384834 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
@@ -165,10 +165,10 @@
#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 0x2
#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12 0x3
#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK GENMASK(2, 0)
-#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB 0x0
-#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 0x1
-#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 0x2
-#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY 0x3
+#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB (0x0 << 4)
+#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 (0x1 << 4)
+#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 (0x2 << 4)
+#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY (0x3 << 4)
#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_MASK GENMASK(5, 4)
#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_CB_FIRST BIT(8)
#define ZYNQMP_DISP_AV_BUF_PALETTE_MEMORY 0x400
--
2.25.1
DPSUB in bridge mode supports multiple input media bus formats.
Announce the list of supported input media bus formats via
drm_bridge.atomic_get_input_bus_fmts callback.
Introduce a set of live input formats, supported by DPSUB.
Rename zynqmp_disp_layer_drm_formats() to zynqmp_disp_layer_formats() to
reflect semantics for both live and non-live layer format lists.
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 76 +++++++++++++++++++++++++++++++++-----
drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +-
drivers/gpu/drm/xlnx/zynqmp_dp.c | 31 ++++++++++++++++
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
4 files changed, 101 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index e6d26ef60e89..abdc3971b193 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -18,6 +18,7 @@
#include <linux/dma/xilinx_dpdma.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
+#include <linux/media-bus-format.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -77,12 +78,14 @@ enum zynqmp_dpsub_layer_mode {
/**
* struct zynqmp_disp_format - Display subsystem format information
* @drm_fmt: DRM format (4CC)
+ * @bus_fmt: Media bus format
* @buf_fmt: AV buffer format
* @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats
* @sf: Scaling factors for color components
*/
struct zynqmp_disp_format {
u32 drm_fmt;
+ u32 bus_fmt;
u32 buf_fmt;
bool swap;
const u32 *sf;
@@ -182,6 +185,12 @@ static const u32 scaling_factors_565[] = {
ZYNQMP_DISP_AV_BUF_5BIT_SF,
};
+static const u32 scaling_factors_666[] = {
+ ZYNQMP_DISP_AV_BUF_6BIT_SF,
+ ZYNQMP_DISP_AV_BUF_6BIT_SF,
+ ZYNQMP_DISP_AV_BUF_6BIT_SF,
+};
+
static const u32 scaling_factors_888[] = {
ZYNQMP_DISP_AV_BUF_8BIT_SF,
ZYNQMP_DISP_AV_BUF_8BIT_SF,
@@ -364,6 +373,41 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
},
};
+/* List of live video layer formats */
+static const struct zynqmp_disp_format avbuf_live_fmts[] = {
+ {
+ .drm_fmt = DRM_FORMAT_RGB565,
+ .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_6 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
+ .sf = scaling_factors_666,
+ }, {
+ .drm_fmt = DRM_FORMAT_RGB888,
+ .bus_fmt = MEDIA_BUS_FMT_RGB888_1X24,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
+ .sf = scaling_factors_888,
+ }, {
+ .drm_fmt = DRM_FORMAT_YUV422,
+ .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
+ .sf = scaling_factors_888,
+ }, {
+ .drm_fmt = DRM_FORMAT_YUV444,
+ .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444,
+ .sf = scaling_factors_888,
+ }, {
+ .drm_fmt = DRM_FORMAT_P210,
+ .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
+ .sf = scaling_factors_101010,
+ },
+};
+
static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg)
{
return readl(disp->avbuf.base + reg);
@@ -883,16 +927,17 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
}
/**
- * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by the layer
+ * zynqmp_disp_layer_formats - Return DRM or media bus formats supported by
+ * the layer
* @layer: The layer
* @num_formats: Pointer to the returned number of formats
*
- * Return: A newly allocated u32 array that stores all the DRM formats
+ * Return: A newly allocated u32 array that stores all DRM or media bus formats
* supported by the layer. The number of formats in the array is returned
* through the num_formats argument.
*/
-u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
- unsigned int *num_formats)
+u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
+ unsigned int *num_formats)
{
unsigned int i;
u32 *formats;
@@ -903,7 +948,9 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
return NULL;
for (i = 0; i < layer->info->num_formats; ++i)
- formats[i] = layer->info->formats[i].drm_fmt;
+ formats[i] = layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE
+ ? layer->info->formats[i].drm_fmt
+ : layer->info->formats[i].bus_fmt;
*num_formats = layer->info->num_formats;
return formats;
@@ -1131,6 +1178,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
.num_channels = 1,
},
};
+ static const struct zynqmp_disp_layer_info live_layer_info = {
+ .formats = avbuf_live_fmts,
+ .num_formats = ARRAY_SIZE(avbuf_live_fmts),
+ .num_channels = 0,
+ };
unsigned int i;
int ret;
@@ -1140,12 +1192,18 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
layer->id = i;
layer->disp = disp;
- layer->info = &layer_info[i];
- /* For now assume dpsub works in either live or non-live mode for both layers.
+
+ /*
+ * For now assume dpsub works in either live or non-live mode for both layers.
* Hybrid mode is not supported yet.
*/
- layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
- : ZYNQMP_DPSUB_LAYER_LIVE;
+ if (disp->dpsub->dma_enabled) {
+ layer->mode = ZYNQMP_DPSUB_LAYER_NONLIVE;
+ layer->info = &layer_info[i];
+ } else {
+ layer->mode = ZYNQMP_DPSUB_LAYER_LIVE;
+ layer->info = &live_layer_info;
+ }
ret = zynqmp_disp_layer_request_dma(disp, layer);
if (ret)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
index 9b8b202224d9..88c285a12e23 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
@@ -50,8 +50,8 @@ int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
bool enable, u32 alpha);
-u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
- unsigned int *num_formats);
+u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
+ unsigned int *num_formats);
void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 4faafdd76798..e3b9eb3d9273 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -22,6 +22,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
+#include <linux/media-bus-format.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -1577,6 +1578,35 @@ static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *brid
return drm_edid_read_ddc(connector, &dp->aux.ddc);
}
+static u32 *zynqmp_dp_bridge_default_bus_fmts(unsigned int *num_input_fmts)
+{
+ u32 *formats = kzalloc(sizeof(*formats), GFP_KERNEL);
+
+ if (formats)
+ *formats = MEDIA_BUS_FMT_FIXED;
+ *num_input_fmts = !!formats;
+
+ return formats;
+}
+
+static u32 *
+zynqmp_dp_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ struct zynqmp_dp *dp = bridge_to_dp(bridge);
+ struct zynqmp_disp_layer *layer;
+
+ layer = zynqmp_dp_disp_connected_live_layer(dp);
+ if (layer)
+ return zynqmp_disp_layer_formats(layer, num_input_fmts);
+ else
+ return zynqmp_dp_bridge_default_bus_fmts(num_input_fmts);
+}
+
static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
.attach = zynqmp_dp_bridge_attach,
.detach = zynqmp_dp_bridge_detach,
@@ -1589,6 +1619,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
.atomic_check = zynqmp_dp_bridge_atomic_check,
.detect = zynqmp_dp_bridge_detect,
.edid_read = zynqmp_dp_bridge_edid_read,
+ .atomic_get_input_bus_fmts = zynqmp_dp_bridge_get_input_bus_fmts,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index 43bf416b33d5..bf9fba01df0e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -152,7 +152,7 @@ static int zynqmp_dpsub_create_planes(struct zynqmp_dpsub *dpsub)
unsigned int num_formats;
u32 *formats;
- formats = zynqmp_disp_layer_drm_formats(layer, &num_formats);
+ formats = zynqmp_disp_layer_formats(layer, &num_formats);
if (!formats)
return -ENOMEM;
--
2.25.1
Add a helper function capturing the first connected live display layer
discovery logic.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 04b6bcac3b07..4faafdd76798 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1276,28 +1276,40 @@ static void zynqmp_dp_encoder_mode_set_stream(struct zynqmp_dp *dp,
* DISP Configuration
*/
+/**
+ * zynqmp_dp_disp_connected_live_layer - Return the first connected live layer
+ * @dp: DisplayPort IP core structure
+ *
+ * Return: The first connected live display layer or NULL if none of the live
+ * layer is connected.
+ */
+static struct zynqmp_disp_layer *
+zynqmp_dp_disp_connected_live_layer(struct zynqmp_dp *dp)
+{
+ if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
+ return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
+ else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
+ return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
+ else
+ return NULL;
+}
+
static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
struct drm_bridge_state *old_bridge_state)
{
- enum zynqmp_dpsub_layer_id layer_id;
struct zynqmp_disp_layer *layer;
const struct drm_format_info *info;
- if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
- layer_id = ZYNQMP_DPSUB_LAYER_VID;
- else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
- layer_id = ZYNQMP_DPSUB_LAYER_GFX;
- else
+ layer = zynqmp_dp_disp_connected_live_layer(dp);
+ if (!layer)
return;
- layer = dp->dpsub->layers[layer_id];
-
/* TODO: Make the format configurable. */
info = drm_format_info(DRM_FORMAT_YUV422);
zynqmp_disp_layer_set_format(layer, info);
zynqmp_disp_layer_enable(layer);
- if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
+ if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
else
zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, false, 0);
@@ -1310,11 +1322,8 @@ static void zynqmp_dp_disp_disable(struct zynqmp_dp *dp,
{
struct zynqmp_disp_layer *layer;
- if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
- layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
- else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
- layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
- else
+ layer = zynqmp_dp_disp_connected_live_layer(dp);
+ if (!layer)
return;
zynqmp_disp_disable(dp->dpsub->disp);
--
2.25.1
Program live video input format according to selected media bus format.
In the bridge mode of operation, DPSUB is connected to FPGA CRTC which
almost certainly supports a single media bus format as its output. Expect
this to be delivered via the new bridge atomic state. Program DPSUB
registers accordingly. Update zynqmp_disp_layer_set_format() API to fit
both live and non-live layer types.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 +-
drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 +++++---
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
4 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 0c2b3f4bffa6..a385d22d428e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
const struct zynqmp_disp_format *fmt)
{
unsigned int i;
- u32 val;
+ u32 val, reg;
- val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
- val &= zynqmp_disp_layer_is_video(layer)
- ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
- : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
- val |= fmt->buf_fmt;
- zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
+ layer->disp_fmt = fmt;
+ if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
+ reg = ZYNQMP_DISP_AV_BUF_FMT;
+ val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
+ val &= zynqmp_disp_layer_is_video(layer)
+ ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
+ : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
+ val |= fmt->buf_fmt;
+ } else {
+ reg = zynqmp_disp_layer_is_video(layer)
+ ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
+ : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
+ val = fmt->buf_fmt;
+ }
+ zynqmp_disp_avbuf_write(disp, reg, val);
for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
- unsigned int reg = zynqmp_disp_layer_is_video(layer)
- ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
- : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
+ reg = zynqmp_disp_layer_is_video(layer)
+ ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
+ : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
}
@@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
*/
/**
- * zynqmp_disp_layer_find_format - Find format information for a DRM format
+ * zynqmp_disp_layer_find_format - Find format information for a DRM or media
+ * bus format
* @layer: The layer
- * @drm_fmt: DRM format to search
+ * @drm_or_bus_format: DRM or media bus format
*
* Search display subsystem format information corresponding to the given DRM
- * format @drm_fmt for the @layer, and return a pointer to the format
- * descriptor.
+ * or media bus format @drm_or_bus_format for the @layer, and return a pointer
+ * to the format descriptor. Search key choice depends on @layer mode, for live
+ * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
+ * zynqmp_disp_format.drm_fmt is used.
*
* Return: A pointer to the format descriptor if found, NULL otherwise
*/
static const struct zynqmp_disp_format *
zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
- u32 drm_fmt)
+ u32 drm_or_bus_format)
{
unsigned int i;
+ const struct zynqmp_disp_format *disp_format;
for (i = 0; i < layer->info->num_formats; i++) {
- if (layer->info->formats[i].drm_fmt == drm_fmt)
- return &layer->info->formats[i];
+ disp_format = &layer->info->formats[i];
+ if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
+ disp_format->bus_fmt == drm_or_bus_format) ||
+ (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
+ disp_format->drm_fmt == drm_or_bus_format))
+ return disp_format;
}
return NULL;
@@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
/**
* zynqmp_disp_layer_set_format - Set the layer format
* @layer: The layer
- * @info: The format info
+ * @drm_or_bus_format: DRM or media bus format
*
* Set the format for @layer to @info. The layer must be disabled.
*/
void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
- const struct drm_format_info *info)
+ u32 drm_or_bus_format)
{
unsigned int i;
- layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
- layer->drm_fmt = info;
+ layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
+ if (WARN_ON(!layer->disp_fmt))
+ return;
zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
+ layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
+ if (!layer->drm_fmt)
+ return;
+
if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
return;
@@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
* Set pconfig for each DMA channel to indicate they're part of a
* video group.
*/
- for (i = 0; i < info->num_planes; i++) {
+ for (i = 0; i < layer->drm_fmt->num_planes; i++) {
struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
struct xilinx_dpdma_peripheral_config pconfig = {
.video_group = true,
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
index 88c285a12e23..9f9a5f50ffbc 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
@@ -55,7 +55,7 @@ u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
- const struct drm_format_info *info);
+ u32 drm_or_bus_format);
int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
struct drm_plane_state *state);
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index e3b9eb3d9273..200e63636006 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
struct drm_bridge_state *old_bridge_state)
{
struct zynqmp_disp_layer *layer;
- const struct drm_format_info *info;
+ struct drm_bridge_state *bridge_state;
+ u32 bus_fmt;
layer = zynqmp_dp_disp_connected_live_layer(dp);
if (!layer)
return;
- /* TODO: Make the format configurable. */
- info = drm_format_info(DRM_FORMAT_YUV422);
- zynqmp_disp_layer_set_format(layer, info);
+ bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
+ old_bridge_state->bridge);
+ if (WARN_ON(!bridge_state))
+ return;
+
+ bus_fmt = bridge_state->input_bus_cfg.format;
+ zynqmp_disp_layer_set_format(layer, bus_fmt);
zynqmp_disp_layer_enable(layer);
if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index bf9fba01df0e..d96b3f3f2e3a 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -111,7 +111,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
if (old_state->fb)
zynqmp_disp_layer_disable(layer);
- zynqmp_disp_layer_set_format(layer, new_state->fb->format);
+ zynqmp_disp_layer_set_format(layer, new_state->fb->format->format);
}
zynqmp_disp_layer_update(layer, new_state);
--
2.25.1
DO NOT MERGE. REFERENCE ONLY.
Add binding for AMD/Xilinx Video Timing Controller and Test Pattern
Generator.
Copy media-bus-formats.h into dt-bindings/media to suplement TPG DT node.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
.../bindings/display/xlnx/xlnx,v-tpg.yaml | 87 ++++++++++
.../devicetree/bindings/display/xlnx/xlnx,vtc.yaml | 65 ++++++++
include/dt-bindings/media/media-bus-format.h | 177 +++++++++++++++++++++
3 files changed, 329 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml
new file mode 100644
index 000000000000..df5ee5b547cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/xlnx/xlnx,v-tpg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Video Test Pattern Generator (TPG)
+
+description:
+ AMD/Xilinx Video Test Pattern Generator IP is a soft IP designed to
+ generate test video signal in AXI4-Stream Video format.
+ https://docs.xilinx.com/r/en-US/pg103-v-tpg
+
+maintainers:
+ - Anatoliy Klymenko <[email protected]>
+
+properties:
+ compatible:
+ description:
+ TPG versions backward-compatible with previous versions should list all
+ compatible versions in the newer to older order.
+ enum: ["xlnx,v-tpg-8.0", "xlnx,v-tpg-8.2"]
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ bus-format:
+ description: Output media bus format, one of MEDIA_BUS_FMT_*
+ maxItems: 1
+
+ xlnx,bridge:
+ description: Reference to Video Timing Controller
+ maxItems: 1
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ description:
+ Connections from and to external components in the video pipeline.
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Sink port connected to downstream video IP.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Source port to connect to optional video signal source.
+
+ required:
+ - port@0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - bus-format
+ - xlnx,bridge
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/media/media-bus-format.h>
+
+ tpg_0: tpg@40050000 {
+ compatible = "xlnx,v-tpg-8.0";
+ reg = <0x40050000 0x10000>;
+ interrupts = <0 89 4>;
+ xlnx,bridge = <&vtc_3>;
+ bus-format = <MEDIA_BUS_FMT_UYVY8_1X16>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ tpg_out: endpoint {
+ remote-endpoint = <&dp_encoder>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,vtc.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,vtc.yaml
new file mode 100644
index 000000000000..51eb12cdcfdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,vtc.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/xlnx/xlnx,vtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device-Tree for Xilinx Video Timing Controller(VTC)
+
+description:
+ Xilinx VTC is a general purpose video timing generator and detector.
+ The input side of this core automatically detects horizontal and
+ vertical synchronization, pulses, polarity, blanking timing and active pixels.
+ While on the output, it generates the horizontal and vertical blanking and
+ synchronization pulses used with a standard video system including support
+ for programmable pulse polarity.
+
+ The core is commonly used with Video in to AXI4-Stream core to detect the
+ format and timing of incoming video data or with AXI4-Stream to Video out core
+ to generate outgoing video timing for downstream sinks like a video monitor.
+
+ For details please refer to
+ https://docs.xilinx.com/r/en-US/pg016_v_tc
+
+maintainers:
+ - Sagar Vishal <sagar.vishal.com>
+
+properties:
+ compatible:
+ const: "xlnx,bridge-v-tc-6.1"
+
+ reg:
+ maxItems: 1
+
+ xlnx,pixels-per-clock:
+ description: Pixels per clock of the stream.
+ enum: [1, 2, 4]
+
+ clocks:
+ minItems: 2
+
+ clock-names:
+ items:
+ - const: clk
+ - const: s_axi_aclk
+
+required:
+ - compatible
+ - reg
+ - xlnx,pixels-per-clock
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ v_tc_0: v_tc@80010000 {
+ clock-names = "clk", "s_axi_aclk";
+ clocks = <&clk_wiz_0>, <&misc_clk_0>;
+ compatible = "xlnx,bridge-v-tc-6.1";
+ xlnx,pixels-per-clock = <1>;
+ reg = <0x80010000 0x10000>;
+ };
+
+...
diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
new file mode 100644
index 000000000000..60fc6e11dabc
--- /dev/null
+++ b/include/dt-bindings/media/media-bus-format.h
@@ -0,0 +1,177 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Media Bus API header
+ *
+ * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_MEDIA_BUS_FORMAT_H
+#define __LINUX_MEDIA_BUS_FORMAT_H
+
+/*
+ * These bus formats uniquely identify data formats on the data bus. Format 0
+ * is reserved, MEDIA_BUS_FMT_FIXED shall be used by host-client pairs, where
+ * the data format is fixed. Additionally, "2X8" means that one pixel is
+ * transferred in two 8-bit samples, "BE" or "LE" specify in which order those
+ * samples are transferred over the bus: "LE" means that the least significant
+ * bits are transferred first, "BE" means that the most significant bits are
+ * transferred first, and "PADHI" and "PADLO" define which bits - low or high,
+ * in the incomplete high byte, are filled with padding bits.
+ *
+ * The bus formats are grouped by type, bus_width, bits per component, samples
+ * per pixel and order of subsamples. Numerical values are sorted using generic
+ * numerical sort order (8 thus comes before 10).
+ *
+ * As their value can't change when a new bus format is inserted in the
+ * enumeration, the bus formats are explicitly given a numerical value. The next
+ * free values for each category are listed below, update them when inserting
+ * new pixel codes.
+ */
+
+#define MEDIA_BUS_FMT_FIXED 0x0001
+
+/* RGB - next is 0x1026 */
+#define MEDIA_BUS_FMT_RGB444_1X12 0x1016
+#define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001
+#define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002
+#define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE 0x1003
+#define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE 0x1004
+#define MEDIA_BUS_FMT_RGB565_1X16 0x1017
+#define MEDIA_BUS_FMT_BGR565_2X8_BE 0x1005
+#define MEDIA_BUS_FMT_BGR565_2X8_LE 0x1006
+#define MEDIA_BUS_FMT_RGB565_2X8_BE 0x1007
+#define MEDIA_BUS_FMT_RGB565_2X8_LE 0x1008
+#define MEDIA_BUS_FMT_RGB666_1X18 0x1009
+#define MEDIA_BUS_FMT_RGB666_2X9_BE 0x1025
+#define MEDIA_BUS_FMT_BGR666_1X18 0x1023
+#define MEDIA_BUS_FMT_RBG888_1X24 0x100e
+#define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015
+#define MEDIA_BUS_FMT_BGR666_1X24_CPADHI 0x1024
+#define MEDIA_BUS_FMT_RGB565_1X24_CPADHI 0x1022
+#define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG 0x1010
+#define MEDIA_BUS_FMT_BGR888_1X24 0x1013
+#define MEDIA_BUS_FMT_BGR888_3X8 0x101b
+#define MEDIA_BUS_FMT_GBR888_1X24 0x1014
+#define MEDIA_BUS_FMT_RGB888_1X24 0x100a
+#define MEDIA_BUS_FMT_RGB888_2X12_BE 0x100b
+#define MEDIA_BUS_FMT_RGB888_2X12_LE 0x100c
+#define MEDIA_BUS_FMT_RGB888_3X8 0x101c
+#define MEDIA_BUS_FMT_RGB888_3X8_DELTA 0x101d
+#define MEDIA_BUS_FMT_RGB888_1X7X4_SPWG 0x1011
+#define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA 0x1012
+#define MEDIA_BUS_FMT_RGB666_1X30_CPADLO 0x101e
+#define MEDIA_BUS_FMT_RGB888_1X30_CPADLO 0x101f
+#define MEDIA_BUS_FMT_ARGB8888_1X32 0x100d
+#define MEDIA_BUS_FMT_RGB888_1X32_PADHI 0x100f
+#define MEDIA_BUS_FMT_RGB101010_1X30 0x1018
+#define MEDIA_BUS_FMT_RGB666_1X36_CPADLO 0x1020
+#define MEDIA_BUS_FMT_RGB888_1X36_CPADLO 0x1021
+#define MEDIA_BUS_FMT_RGB121212_1X36 0x1019
+#define MEDIA_BUS_FMT_RGB161616_1X48 0x101a
+
+/* YUV (including grey) - next is 0x202f */
+#define MEDIA_BUS_FMT_Y8_1X8 0x2001
+#define MEDIA_BUS_FMT_UV8_1X8 0x2015
+#define MEDIA_BUS_FMT_UYVY8_1_5X8 0x2002
+#define MEDIA_BUS_FMT_VYUY8_1_5X8 0x2003
+#define MEDIA_BUS_FMT_YUYV8_1_5X8 0x2004
+#define MEDIA_BUS_FMT_YVYU8_1_5X8 0x2005
+#define MEDIA_BUS_FMT_UYVY8_2X8 0x2006
+#define MEDIA_BUS_FMT_VYUY8_2X8 0x2007
+#define MEDIA_BUS_FMT_YUYV8_2X8 0x2008
+#define MEDIA_BUS_FMT_YVYU8_2X8 0x2009
+#define MEDIA_BUS_FMT_Y10_1X10 0x200a
+#define MEDIA_BUS_FMT_Y10_2X8_PADHI_LE 0x202c
+#define MEDIA_BUS_FMT_UYVY10_2X10 0x2018
+#define MEDIA_BUS_FMT_VYUY10_2X10 0x2019
+#define MEDIA_BUS_FMT_YUYV10_2X10 0x200b
+#define MEDIA_BUS_FMT_YVYU10_2X10 0x200c
+#define MEDIA_BUS_FMT_Y12_1X12 0x2013
+#define MEDIA_BUS_FMT_UYVY12_2X12 0x201c
+#define MEDIA_BUS_FMT_VYUY12_2X12 0x201d
+#define MEDIA_BUS_FMT_YUYV12_2X12 0x201e
+#define MEDIA_BUS_FMT_YVYU12_2X12 0x201f
+#define MEDIA_BUS_FMT_Y14_1X14 0x202d
+#define MEDIA_BUS_FMT_Y16_1X16 0x202e
+#define MEDIA_BUS_FMT_UYVY8_1X16 0x200f
+#define MEDIA_BUS_FMT_VYUY8_1X16 0x2010
+#define MEDIA_BUS_FMT_YUYV8_1X16 0x2011
+#define MEDIA_BUS_FMT_YVYU8_1X16 0x2012
+#define MEDIA_BUS_FMT_YDYUYDYV8_1X16 0x2014
+#define MEDIA_BUS_FMT_UYVY10_1X20 0x201a
+#define MEDIA_BUS_FMT_VYUY10_1X20 0x201b
+#define MEDIA_BUS_FMT_YUYV10_1X20 0x200d
+#define MEDIA_BUS_FMT_YVYU10_1X20 0x200e
+#define MEDIA_BUS_FMT_VUY8_1X24 0x2024
+#define MEDIA_BUS_FMT_YUV8_1X24 0x2025
+#define MEDIA_BUS_FMT_UYYVYY8_0_5X24 0x2026
+#define MEDIA_BUS_FMT_UYVY12_1X24 0x2020
+#define MEDIA_BUS_FMT_VYUY12_1X24 0x2021
+#define MEDIA_BUS_FMT_YUYV12_1X24 0x2022
+#define MEDIA_BUS_FMT_YVYU12_1X24 0x2023
+#define MEDIA_BUS_FMT_YUV10_1X30 0x2016
+#define MEDIA_BUS_FMT_UYYVYY10_0_5X30 0x2027
+#define MEDIA_BUS_FMT_AYUV8_1X32 0x2017
+#define MEDIA_BUS_FMT_UYYVYY12_0_5X36 0x2028
+#define MEDIA_BUS_FMT_YUV12_1X36 0x2029
+#define MEDIA_BUS_FMT_YUV16_1X48 0x202a
+#define MEDIA_BUS_FMT_UYYVYY16_0_5X48 0x202b
+
+/* Bayer - next is 0x3021 */
+#define MEDIA_BUS_FMT_SBGGR8_1X8 0x3001
+#define MEDIA_BUS_FMT_SGBRG8_1X8 0x3013
+#define MEDIA_BUS_FMT_SGRBG8_1X8 0x3002
+#define MEDIA_BUS_FMT_SRGGB8_1X8 0x3014
+#define MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8 0x3015
+#define MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8 0x3016
+#define MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8 0x3017
+#define MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8 0x3018
+#define MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8 0x300b
+#define MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8 0x300c
+#define MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8 0x3009
+#define MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8 0x300d
+#define MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE 0x3003
+#define MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE 0x3004
+#define MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE 0x3005
+#define MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE 0x3006
+#define MEDIA_BUS_FMT_SBGGR10_1X10 0x3007
+#define MEDIA_BUS_FMT_SGBRG10_1X10 0x300e
+#define MEDIA_BUS_FMT_SGRBG10_1X10 0x300a
+#define MEDIA_BUS_FMT_SRGGB10_1X10 0x300f
+#define MEDIA_BUS_FMT_SBGGR12_1X12 0x3008
+#define MEDIA_BUS_FMT_SGBRG12_1X12 0x3010
+#define MEDIA_BUS_FMT_SGRBG12_1X12 0x3011
+#define MEDIA_BUS_FMT_SRGGB12_1X12 0x3012
+#define MEDIA_BUS_FMT_SBGGR14_1X14 0x3019
+#define MEDIA_BUS_FMT_SGBRG14_1X14 0x301a
+#define MEDIA_BUS_FMT_SGRBG14_1X14 0x301b
+#define MEDIA_BUS_FMT_SRGGB14_1X14 0x301c
+#define MEDIA_BUS_FMT_SBGGR16_1X16 0x301d
+#define MEDIA_BUS_FMT_SGBRG16_1X16 0x301e
+#define MEDIA_BUS_FMT_SGRBG16_1X16 0x301f
+#define MEDIA_BUS_FMT_SRGGB16_1X16 0x3020
+
+/* JPEG compressed formats - next is 0x4002 */
+#define MEDIA_BUS_FMT_JPEG_1X8 0x4001
+
+/* Vendor specific formats - next is 0x5002 */
+
+/* S5C73M3 sensor specific interleaved UYVY and JPEG */
+#define MEDIA_BUS_FMT_S5C_UYVY_JPEG_1X8 0x5001
+
+/* HSV - next is 0x6002 */
+#define MEDIA_BUS_FMT_AHSV8888_1X32 0x6001
+
+/*
+ * This format should be used when the same driver handles
+ * both sides of the link and the bus format is a fixed
+ * metadata format that is not configurable from userspace.
+ * Width and height will be set to 0 for this format.
+ */
+#define MEDIA_BUS_FMT_METADATA_FIXED 0x7001
+
+#endif /* __LINUX_MEDIA_BUS_FORMAT_H */
--
2.25.1
Add optional drm_crtc_helper_funcs.select_output_bus_format callback. This
callback allows to negotiate compatible media bus format on the link
between CRTC and connected DRM encoder or DRM bridge chain. A good usage
example is the CRTC implemented as FPGA soft IP. This kind of CRTC will
most certainly support a single output media bus format, as supporting
multiple runtime options consumes extra FPGA resources. A variety of
options for the FPGA designs are usually achieved by synthesizing IP with
different parameters.
Add drm_helper_crtc_select_output_bus_format that wraps
drm_crtc_helper_funcs.select_output_bus_format.
Incorporate select_output_bus_format callback into the format negotiation
stage to fix the input bus format of the first DRM bridge in the chain.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/drm_bridge.c | 14 +++++++++++--
drivers/gpu/drm/drm_crtc_helper.c | 36 ++++++++++++++++++++++++++++++++
include/drm/drm_crtc_helper.h | 5 +++++
include/drm/drm_modeset_helper_vtables.h | 30 ++++++++++++++++++++++++++
4 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 521a71c61b16..955ca108cd4b 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -28,6 +28,7 @@
#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_debugfs.h>
#include <drm/drm_edid.h>
#include <drm/drm_encoder.h>
@@ -879,7 +880,8 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
unsigned int i, num_in_bus_fmts = 0;
struct drm_bridge_state *cur_state;
struct drm_bridge *prev_bridge;
- u32 *in_bus_fmts;
+ struct drm_crtc *crtc = crtc_state->crtc;
+ u32 *in_bus_fmts, in_fmt;
int ret;
prev_bridge = drm_bridge_get_prev_bridge(cur_bridge);
@@ -933,7 +935,15 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
return -ENOMEM;
if (first_bridge == cur_bridge) {
- cur_state->input_bus_cfg.format = in_bus_fmts[0];
+ in_fmt = drm_helper_crtc_select_output_bus_format(crtc,
+ crtc_state,
+ in_bus_fmts,
+ num_in_bus_fmts);
+ if (!in_fmt) {
+ kfree(in_bus_fmts);
+ return -ENOTSUPP;
+ }
+ cur_state->input_bus_cfg.format = in_fmt;
cur_state->output_bus_cfg.format = out_bus_fmt;
kfree(in_bus_fmts);
return 0;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 2dafc39a27cb..f2e12a3c4e5f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct drm_device *dev)
return ret;
}
EXPORT_SYMBOL(drm_helper_force_disable_all);
+
+/**
+ * drm_helper_crtc_select_output_bus_format - Select output media bus format
+ * @crtc: The CRTC to query
+ * @crtc_state: The new CRTC state
+ * @supported_fmts: List of media bus format options to pick from
+ * @num_supported_fmts: Number of media bus formats in @supported_fmts list
+ *
+ * Encoder drivers may call this helper to give the connected CRTC a chance to
+ * select compatible or preffered media bus format to use over the CRTC encoder
+ * link. Encoders should provide list of supported input MEDIA_BUS_FMT_* for
+ * CRTC to pick from. CRTC driver is expected to select preferred media bus
+ * format from the list and, once enabled, generate the signal accordingly.
+ *
+ * Returns:
+ * Selected preferred media bus format or 0 if CRTC does not support any from
+ * @supported_fmts list.
+ */
+u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts)
+{
+ if (!crtc || !supported_fmts || !num_supported_fmts)
+ return 0;
+
+ if (!crtc->helper_private ||
+ !crtc->helper_private->select_output_bus_format)
+ return supported_fmts[0];
+
+ return crtc->helper_private->select_output_bus_format(crtc,
+ crtc_state,
+ supported_fmts,
+ num_supported_fmts);
+}
+EXPORT_SYMBOL(drm_helper_crtc_select_output_bus_format);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 8c886fc46ef2..b7eb52f3ce41 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -38,6 +38,7 @@
struct drm_atomic_state;
struct drm_connector;
struct drm_crtc;
+struct drm_crtc_state;
struct drm_device;
struct drm_display_mode;
struct drm_encoder;
@@ -61,5 +62,9 @@ int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
void drm_helper_resume_force_mode(struct drm_device *dev);
int drm_helper_force_disable_all(struct drm_device *dev);
+u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts);
#endif
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 881b03e4dc28..6d5a081e21a4 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -489,6 +489,36 @@ struct drm_crtc_helper_funcs {
bool in_vblank_irq, int *vpos, int *hpos,
ktime_t *stime, ktime_t *etime,
const struct drm_display_mode *mode);
+
+ /**
+ * @select_output_bus_format
+ *
+ * Called by the connected DRM encoder to negotiate input media bus
+ * format. CRTC is expected to pick preferable media formats from the
+ * list provided by the DRM encoder.
+ *
+ * This callback is optional.
+ *
+ * Parameters:
+ *
+ * crtc:
+ * The CRTC.
+ * crcs_state:
+ * New CRTC state.
+ * supported_fmts:
+ * List of input bus formats supported by the encoder.
+ * num_supported_fmts:
+ * Number of formats in the list.
+ *
+ * Returns:
+ *
+ * Preferred bus format from the list or 0 if CRTC doesn't support any
+ * from the provided list.
+ */
+ u32 (*select_output_bus_format)(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *supported_fmts,
+ unsigned int num_supported_fmts);
};
/**
--
2.25.1
Avoid usage of global zynqmp_dpsub.dma_enabled flag in DPSUB layer
context. This flag signals whether the driver runs in DRM CRTC or DRM
bridge mode, assuming that all display layers share the same live or
non-live mode of operation. Using per-layer mode instead of global flag
will simplify future support of the hybrid scenario.
Remove redundant checks in DMA request/release contexts as
zynqmp_disp_layer.info is well-defined for all layer types, including the
correct number of DMA channels required for each particular layer.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index abdc3971b193..0c2b3f4bffa6 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -980,7 +980,7 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
{
unsigned int i;
- if (layer->disp->dpsub->dma_enabled) {
+ if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
for (i = 0; i < layer->drm_fmt->num_planes; i++)
dmaengine_terminate_sync(layer->dmas[i].chan);
}
@@ -1006,7 +1006,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
- if (!layer->disp->dpsub->dma_enabled)
+ if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
return;
/*
@@ -1044,7 +1044,7 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
const struct drm_format_info *info = layer->drm_fmt;
unsigned int i;
- if (!layer->disp->dpsub->dma_enabled)
+ if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
return 0;
for (i = 0; i < info->num_planes; i++) {
@@ -1094,9 +1094,6 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
{
unsigned int i;
- if (!layer->info || !disp->dpsub->dma_enabled)
- return;
-
for (i = 0; i < layer->info->num_channels; i++) {
struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
@@ -1137,9 +1134,6 @@ static int zynqmp_disp_layer_request_dma(struct zynqmp_disp *disp,
unsigned int i;
int ret;
- if (!disp->dpsub->dma_enabled)
- return 0;
-
for (i = 0; i < layer->info->num_channels; i++) {
struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
char dma_channel_name[16];
--
2.25.1
DO NOT MERGE. REFERENCE ONLY.
Add CRTC driver based on AMD/Xilinx Video Test Pattern Generator IP. TPG
based FPGA design represents minimalistic harness useful for testing links
between FPGA based CRTC and external DRM encoders, both FPGA and hardened
IP based.
Add driver for AMD/Xilinx Video Timing Controller. The VTC, working in
generator mode, suplements TPG with video timing signals.
Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/Kconfig | 21 +
drivers/gpu/drm/xlnx/Makefile | 4 +
drivers/gpu/drm/xlnx/xlnx_tpg.c | 846 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/xlnx/xlnx_vtc.c | 452 +++++++++++++++++++
drivers/gpu/drm/xlnx/xlnx_vtc.h | 101 +++++
drivers/gpu/drm/xlnx/xlnx_vtc_list.c | 160 +++++++
6 files changed, 1584 insertions(+)
diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index 68ee897de9d7..c40e98c1a5e6 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -15,3 +15,24 @@ config DRM_ZYNQMP_DPSUB
This is a DRM/KMS driver for ZynqMP DisplayPort controller. Choose
this option if you have a Xilinx ZynqMP SoC with DisplayPort
subsystem.
+
+config DRM_XLNX_BRIDGE_VTC
+ bool "Xilinx DRM VTC Driver"
+ depends on OF
+ help
+ DRM brige driver for Xilinx Video Timing Controller. Choose
+ this option to make VTC a part of the CRTC in display pipeline.
+ Currently the support is added to the Xilinx Video Mixer and
+ Xilinx PL display CRTC drivers. This driver provides ability
+ to generate timings through the bridge layer.
+
+config DRM_XLNX_TPG
+ bool "Xilinx DRM TPG Driver"
+ depends on DRM && OF
+ select DRM_XLNX_BRIDGE_VTC
+ select VIDEOMODE_HELPERS
+ help
+ CRTC driver based on AMD/Xilinx Test Pattern Generator IP. Choose
+ this driver to enable Test Pattern Generator CRTC. This driver
+ implements simplistic CRTC with the single plane and is perfect for
+ testing PL to PS and PL to PL display output pipelines.
diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
index ea1422a39502..26fb3ad21fa9 100644
--- a/drivers/gpu/drm/xlnx/Makefile
+++ b/drivers/gpu/drm/xlnx/Makefile
@@ -1,2 +1,6 @@
zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o zynqmp_kms.o
obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
+xlnx-tpg-objs := xlnx_tpg.o
+xlnx-tpg-$(CONFIG_DRM_XLNX_BRIDGE_VTC) += xlnx_vtc_list.o
+obj-$(CONFIG_DRM_XLNX_TPG) += xlnx-tpg.o
+obj-$(CONFIG_DRM_XLNX_BRIDGE_VTC) += xlnx_vtc.o
diff --git a/drivers/gpu/drm/xlnx/xlnx_tpg.c b/drivers/gpu/drm/xlnx/xlnx_tpg.c
new file mode 100644
index 000000000000..3056bf278414
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_tpg.c
@@ -0,0 +1,846 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx logicore test pattern generator driver
+ *
+ * Copyright (C) 2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Anatoliy Klymenko <[email protected]>
+ *
+ * This driver introduces support for the test CRTC based on AMD/Xilinx
+ * Test Pattern Generator IP. The main goal of the driver is to enable
+ * simplistic FPGA design that could be used to test FPGA CRTC to external
+ * encoder IP connectivity.
+ * Reference: https://docs.xilinx.com/r/en-US/pg103-v-tpg
+ */
+
+#include "xlnx_vtc.h"
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <linux/media-bus-format.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_vblank.h>
+#include <linux/component.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <video/videomode.h>
+
+#define DRIVER_NAME "xlnx-tpg"
+#define DRIVER_DESC "Xilinx TPG DRM KMS Driver"
+#define DRIVER_DATE "20240307"
+#define DRIVER_MAJOR 1
+#define DRIVER_MINOR 0
+
+#define XLNX_TPG_CONTROL 0x0000
+#define XLNX_TPG_GLOBAL_IRQ_EN 0x0004
+#define XLNX_TPG_IP_IRQ_EN 0x0008
+#define XLNX_TPG_IP_IRQ_STATUS 0x000C
+#define XLNX_TPG_ACTIVE_HEIGHT 0x0010
+#define XLNX_TPG_ACTIVE_WIDTH 0x0018
+#define XLNX_TPG_PATTERN_ID 0x0020
+#define XLNX_TPG_COLOR_FORMAT 0x0040
+
+#define XLNX_TPG_IP_IRQ_AP_DONE BIT(0)
+
+#define XLNX_TPG_START BIT(0)
+#define XLNX_TPG_AUTO_RESTART BIT(7)
+
+enum xlnx_tpg_pattern {
+ XTPG_PAT_HORIZONTAL_RAMP = 0x1,
+ XTPG_PAT_VERTICAL_RAMP,
+ XTPG_PAT_TEMPORAL_RAMP,
+ XTPG_PAT_SOLID_RED,
+ XTPG_PAT_SOLID_GREEN,
+ XTPG_PAT_SOLID_BLUE,
+ XTPG_PAT_SOLID_BLACK,
+ XTPG_PAT_SOLID_WHITE,
+ XTPG_PAT_COLOR_BARS,
+ XTPG_PAT_ZONE_PLATE,
+ XTPG_PAT_TARTAN_COLOR_BARS,
+ XTPG_PAT_CROSS_HATCH,
+ XTPG_PAT_COLOR_SWEEP,
+ XTPG_PAT_COMBO_RAMP,
+ XTPG_PAT_CHECKER_BOARD,
+ XTPG_PAT_DP_COLOR_RAMP,
+ XTPG_PAT_DP_VERTICAL_LINES,
+ XTPG_PAT_DP_COLOR_SQUARE,
+};
+
+static const struct drm_prop_enum_list xtpg_pattern_list[] = {
+ { XTPG_PAT_HORIZONTAL_RAMP, "horizontal-ramp" },
+ { XTPG_PAT_VERTICAL_RAMP, "vertical-ramp" },
+ { XTPG_PAT_TEMPORAL_RAMP, "temporal-ramp" },
+ { XTPG_PAT_SOLID_RED, "red" },
+ { XTPG_PAT_SOLID_GREEN, "green" },
+ { XTPG_PAT_SOLID_BLUE, "blue" },
+ { XTPG_PAT_SOLID_BLACK, "black" },
+ { XTPG_PAT_SOLID_WHITE, "white" },
+ { XTPG_PAT_COLOR_BARS, "color-bars" },
+ { XTPG_PAT_ZONE_PLATE, "zone-plate" },
+ { XTPG_PAT_TARTAN_COLOR_BARS, "tartan-color-bars" },
+ { XTPG_PAT_CROSS_HATCH, "cross-hatch" },
+ { XTPG_PAT_COLOR_SWEEP, "color-sweep" },
+ { XTPG_PAT_COMBO_RAMP, "combo-ramp" },
+ { XTPG_PAT_CHECKER_BOARD, "checker-board" },
+ { XTPG_PAT_DP_COLOR_RAMP, "dp-color-ramp" },
+ { XTPG_PAT_DP_VERTICAL_LINES, "dp-vertical-lines" },
+ { XTPG_PAT_DP_COLOR_SQUARE, "dp-color-square" },
+};
+
+enum xlnx_tpg_format {
+ XTPG_FMT_RGB = 0x0,
+ XTPG_FMT_YUV_444,
+ XTPG_FMT_YUV_422,
+ XTPG_FMT_YUV_420,
+ XTPG_FMT_INVALID,
+};
+
+struct xlnx_tpg;
+
+/**
+ * struct xlnx_tpg_drm - TPG CRTC DRM/KMS data
+ * @tpg: Back pointer to parent TPG
+ * @dev: DRM device
+ * @crtc: DRM CRTC
+ * @plane: DRM primary plane
+ * @encoder: DRM encoder
+ * @connector: DRM connector
+ * @pattern_prop: DRM property representing TPG video pattern
+ * @event: Pending DRM VBLANK event
+ */
+struct xlnx_tpg_drm {
+ struct xlnx_tpg *tpg;
+ struct drm_device dev;
+ struct drm_crtc crtc;
+ struct drm_plane plane;
+ struct drm_encoder encoder;
+ struct drm_connector *connector;
+ struct drm_property *pattern_prop;
+ struct drm_pending_vblank_event *event;
+};
+
+/**
+ * struct xlnx_tpg_drm - Test Pattern Generator data
+ * @pdev: Platform device
+ * @drm: TPG DRM data
+ * @vtc: Video timing controller interface
+ * @disp_bridge: DRM display bridge
+ * @regs: Mapped TPG IP register space
+ * @irq: TPG IRQ
+ * @output_bus_format: Chosen TPG output bus format
+ * @color_format: TPG color format
+ */
+struct xlnx_tpg {
+ struct platform_device *pdev;
+ struct xlnx_tpg_drm *drm;
+ struct xlnx_vtc_iface *vtc;
+ struct drm_bridge *disp_bridge;
+ void __iomem *regs;
+ int irq;
+ u32 output_bus_format;
+ enum xlnx_tpg_format color_format;
+};
+
+static inline struct xlnx_tpg *crtc_to_tpg(struct drm_crtc *crtc)
+{
+ return container_of(crtc, struct xlnx_tpg_drm, crtc)->tpg;
+}
+
+static inline struct xlnx_tpg *plane_to_tpg(struct drm_plane *plane)
+{
+ return container_of(plane, struct xlnx_tpg_drm, plane)->tpg;
+}
+
+static inline struct xlnx_tpg *encoder_to_tpg(struct drm_encoder *encoder)
+{
+ return container_of(encoder, struct xlnx_tpg_drm, encoder)->tpg;
+}
+
+struct xlnx_tpg_format_map {
+ u32 bus_format;
+ enum xlnx_tpg_format color_format;
+};
+
+/**
+ * xlnx_tpg_bus_to_color_format - Map media bus format to TPG color format
+ * @bus_format: Media bus format
+ *
+ * Return: TPG color format that matches @bus_format or XTPG_FMT_INVALID if
+ * input media bus format is not supported
+ */
+static enum xlnx_tpg_format xlnx_tpg_bus_to_color_format(u32 bus_format)
+{
+ static const struct xlnx_tpg_format_map format_map[] = {
+ {
+ .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+ .color_format = XTPG_FMT_RGB,
+ }, {
+ .bus_format = MEDIA_BUS_FMT_RBG888_1X24,
+ .color_format = XTPG_FMT_RGB,
+ }, {
+ .bus_format = MEDIA_BUS_FMT_UYVY8_1X16,
+ .color_format = XTPG_FMT_YUV_422,
+ }, {
+ .bus_format = MEDIA_BUS_FMT_VUY8_1X24,
+ .color_format = XTPG_FMT_YUV_444,
+ }, {
+ .bus_format = MEDIA_BUS_FMT_UYVY10_1X20,
+ .color_format = XTPG_FMT_YUV_422,
+ },
+ };
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(format_map); ++i)
+ if (format_map[i].bus_format == bus_format)
+ return format_map[i].color_format;
+
+ return XTPG_FMT_INVALID;
+}
+
+/* -----------------------------------------------------------------------------
+ * TPG IP ops
+ */
+
+static void xlnx_tpg_write(struct xlnx_tpg *tpg, int offset, u32 val)
+{
+ writel(val, tpg->regs + offset);
+}
+
+static u32 xlnx_tpg_read(struct xlnx_tpg *tpg, int offset)
+{
+ return readl(tpg->regs + offset);
+}
+
+/**
+ * xlnx_tpg_set_dimensions - Set TPG output signal dimensions
+ * @tpg: The TPG
+ * @w: Output video frame width
+ * @h: Output video frame height
+ */
+static void xlnx_tpg_set_dimensions(struct xlnx_tpg *tpg, u16 w, u16 h)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_ACTIVE_WIDTH, w);
+ xlnx_tpg_write(tpg, XLNX_TPG_ACTIVE_HEIGHT, h);
+}
+
+/**
+ * xlnx_tpg_set_pattern - Set TPG output video pattern
+ * @tpg: The TPG
+ * @pattern: The pattern
+ */
+static void xlnx_tpg_set_pattern(struct xlnx_tpg *tpg, enum xlnx_tpg_pattern pattern)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_PATTERN_ID, pattern);
+}
+
+/**
+ * xlnx_tpg_get_pattern - Get programmed TPG output video pattern
+ * @tpg: The TPG
+ *
+ * Return: Video signal pattern programmed
+ */
+static enum xlnx_tpg_pattern xlnx_tpg_get_pattern(struct xlnx_tpg *tpg)
+{
+ return xlnx_tpg_read(tpg, XLNX_TPG_PATTERN_ID);
+}
+
+/**
+ * xlnx_tpg_set_format - Set TPG output video color format
+ * @tpg: The TPG
+ * @format: Color format to program
+ */
+static void xlnx_tpg_set_format(struct xlnx_tpg *tpg, enum xlnx_tpg_format format)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_COLOR_FORMAT, format);
+}
+
+/**
+ * xlnx_tpg_start - Start generation of the video signal
+ * @tpg: The TPG
+ */
+static void xlnx_tpg_start(struct xlnx_tpg *tpg)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_CONTROL, XLNX_TPG_START | XLNX_TPG_AUTO_RESTART);
+}
+
+/**
+ * xlnx_tpg_enable_irq - Enable generation of the frame done interrupts
+ * @tpg: The TPG
+ */
+static void xlnx_tpg_enable_irq(struct xlnx_tpg *tpg)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_GLOBAL_IRQ_EN, 1);
+ xlnx_tpg_write(tpg, XLNX_TPG_IP_IRQ_EN, 1);
+}
+
+/**
+ * xlnx_tpg_disable_irq - Disable generation of the frame done interrupts
+ * @tpg: The TPG
+ */
+static void xlnx_tpg_disable_irq(struct xlnx_tpg *tpg)
+{
+ xlnx_tpg_write(tpg, XLNX_TPG_GLOBAL_IRQ_EN, 0);
+ xlnx_tpg_write(tpg, XLNX_TPG_IP_IRQ_EN, 0);
+}
+
+static irqreturn_t xlnx_tpg_irq_handler(int irq, void *data)
+{
+ struct xlnx_tpg *tpg = data;
+ struct drm_crtc *crtc = &tpg->drm->crtc;
+ struct drm_pending_vblank_event *event;
+ unsigned long flags;
+ u32 status = xlnx_tpg_read(tpg, XLNX_TPG_IP_IRQ_STATUS);
+
+ xlnx_tpg_write(tpg, XLNX_TPG_IP_IRQ_STATUS, status);
+
+ status &= XLNX_TPG_IP_IRQ_AP_DONE;
+ if (!status)
+ return IRQ_NONE;
+
+ drm_crtc_handle_vblank(crtc);
+
+ /* Finish page flip */
+ spin_lock_irqsave(&crtc->dev->event_lock, flags);
+ event = tpg->drm->event;
+ tpg->drm->event = NULL;
+ if (event) {
+ drm_crtc_send_vblank_event(crtc, event);
+ drm_crtc_vblank_put(crtc);
+ }
+ spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * xlnx_tpg_setup_irq - Setup TPG interrupt
+ * @tpg: The TPG
+ *
+ * Return: 0 on success or error code
+ */
+static int xlnx_tpg_setup_irq(struct xlnx_tpg *tpg)
+{
+ struct device_node *node = tpg->pdev->dev.of_node;
+ int ret;
+
+ tpg->irq = irq_of_parse_and_map(node, 0);
+ if (!tpg->irq) {
+ dev_err(&tpg->pdev->dev, "failed to parse irq\n");
+ return -EINVAL;
+ }
+
+ ret = devm_request_irq(&tpg->pdev->dev, tpg->irq, xlnx_tpg_irq_handler,
+ IRQF_SHARED, "xlnx-tpg", tpg);
+ if (ret < 0) {
+ dev_err(&tpg->pdev->dev, "failed to request irq\n");
+ return ret;
+ }
+
+ xlnx_tpg_enable_irq(tpg);
+
+ return 0;
+}
+
+/**
+ * xlnx_tpg_map_resources - Map TPG register space
+ * @tpg: The TPG
+ *
+ * Return: 0 on success or error code
+ */
+static int xlnx_tpg_map_resources(struct xlnx_tpg *tpg)
+{
+ struct device_node *node = tpg->pdev->dev.of_node;
+ struct resource res;
+ int ret;
+
+ ret = of_address_to_resource(node, 0, &res);
+ if (ret < 0) {
+ dev_err(&tpg->pdev->dev, "failed to parse resource\n");
+ return ret;
+ }
+
+ tpg->regs = devm_ioremap_resource(&tpg->pdev->dev, &res);
+ if (IS_ERR(tpg->regs)) {
+ ret = PTR_ERR(tpg->regs);
+ dev_err(&tpg->pdev->dev, "failed to map register space\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+/* -----------------------------------------------------------------------------
+ * DRM plane
+ */
+
+static void xlnx_tpg_plane_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state *state)
+{
+ struct xlnx_tpg *tpg = plane_to_tpg(plane);
+ struct drm_crtc *crtc = &tpg->drm->crtc;
+
+ drm_crtc_vblank_on(crtc);
+ if (crtc->state->event) {
+ /* Consume the flip_done event from atomic helper */
+ crtc->state->event->pipe = drm_crtc_index(crtc);
+ drm_crtc_vblank_get(crtc);
+ tpg->drm->event = crtc->state->event;
+ crtc->state->event = NULL;
+ }
+}
+
+static int xlnx_tpg_plane_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_state *state)
+{
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_new_plane_state(state, plane);
+ struct xlnx_tpg *tpg = plane_to_tpg(plane);
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, &tpg->drm->crtc);
+
+ return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, false);
+}
+
+static const struct drm_plane_helper_funcs xlnx_tpg_plane_helper_funcs = {
+ .prepare_fb = drm_gem_plane_helper_prepare_fb,
+ .atomic_check = xlnx_tpg_plane_atomic_check,
+ .atomic_update = xlnx_tpg_plane_atomic_update,
+};
+
+static bool xlnx_tpg_format_mod_supported(struct drm_plane *plane,
+ uint32_t format,
+ uint64_t modifier)
+{
+ return modifier == DRM_FORMAT_MOD_LINEAR;
+}
+
+static int xlnx_tpg_plane_set_property(struct drm_plane *plane,
+ struct drm_plane_state *state,
+ struct drm_property *property,
+ u64 val)
+{
+ struct xlnx_tpg *tpg = plane_to_tpg(plane);
+
+ if (property == tpg->drm->pattern_prop)
+ xlnx_tpg_set_pattern(tpg, val);
+ else
+ return -EINVAL;
+ return 0;
+}
+
+static int xlnx_tpg_plane_get_property(struct drm_plane *plane,
+ const struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t *val)
+{
+ struct xlnx_tpg *tpg = plane_to_tpg(plane);
+
+ if (property == tpg->drm->pattern_prop)
+ *val = xlnx_tpg_get_pattern(tpg);
+ else
+ return -EINVAL;
+ return 0;
+}
+
+static const struct drm_plane_funcs xlnx_tpg_plane_funcs = {
+ .update_plane = drm_atomic_helper_update_plane,
+ .disable_plane = drm_atomic_helper_disable_plane,
+ .destroy = drm_plane_cleanup,
+ .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 = xlnx_tpg_format_mod_supported,
+ .atomic_set_property = xlnx_tpg_plane_set_property,
+ .atomic_get_property = xlnx_tpg_plane_get_property,
+};
+
+/**
+ * xlnx_tpg_create_properties - Create TPG DRM properties
+ * @tpg: The TPG
+ */
+static void xlnx_tpg_create_properties(struct xlnx_tpg *tpg)
+{
+ struct drm_device *drm = &tpg->drm->dev;
+ struct drm_mode_object *obj = &tpg->drm->plane.base;
+
+ tpg->drm->pattern_prop = drm_property_create_enum(drm, 0, "pattern", xtpg_pattern_list,
+ ARRAY_SIZE(xtpg_pattern_list));
+ drm_object_attach_property(obj, tpg->drm->pattern_prop, XTPG_PAT_COLOR_BARS);
+ xlnx_tpg_set_pattern(tpg, XTPG_PAT_COLOR_BARS);
+}
+
+/* -----------------------------------------------------------------------------
+ * DRM CRTC
+ */
+
+static enum drm_mode_status xlnx_tpg_crtc_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode)
+{
+ return MODE_OK;
+}
+
+static int xlnx_tpg_crtc_check(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ int ret;
+
+ if (!crtc_state->enable)
+ goto out;
+
+ ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+ if (ret)
+ return ret;
+
+out:
+ return drm_atomic_add_affected_planes(state, crtc);
+}
+
+static void xlnx_tpg_crtc_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct videomode vm;
+ struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+ struct xlnx_tpg *tpg = crtc_to_tpg(crtc);
+
+ if (tpg->vtc) {
+ drm_display_mode_to_videomode(mode, &vm);
+ xlnx_vtc_iface_set_timing(tpg->vtc, &vm);
+ xlnx_vtc_iface_enable(tpg->vtc);
+ }
+
+ xlnx_tpg_set_dimensions(tpg, mode->hdisplay, mode->vdisplay);
+
+ xlnx_tpg_set_format(tpg, tpg->color_format);
+ xlnx_tpg_start(tpg);
+}
+
+static void xlnx_tpg_crtc_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct xlnx_tpg *tpg = crtc_to_tpg(crtc);
+
+ if (tpg->vtc)
+ xlnx_vtc_iface_disable(tpg->vtc);
+ if (crtc->state->event) {
+ complete_all(crtc->state->event->base.completion);
+ crtc->state->event = NULL;
+ }
+ drm_crtc_vblank_off(crtc);
+}
+
+static u32 xlnx_tpg_crtc_select_output_bus_format(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state,
+ const u32 *in_bus_fmts,
+ unsigned int num_in_bus_fmts)
+{
+ struct xlnx_tpg *tpg = crtc_to_tpg(crtc);
+ unsigned int i;
+
+ for (i = 0; i < num_in_bus_fmts; ++i)
+ if (in_bus_fmts[i] == tpg->output_bus_format)
+ return tpg->output_bus_format;
+
+ return 0;
+}
+
+static const struct drm_crtc_helper_funcs xlnx_tpg_crtc_helper_funcs = {
+ .mode_valid = xlnx_tpg_crtc_mode_valid,
+ .atomic_check = xlnx_tpg_crtc_check,
+ .atomic_enable = xlnx_tpg_crtc_enable,
+ .atomic_disable = xlnx_tpg_crtc_disable,
+ .select_output_bus_format = xlnx_tpg_crtc_select_output_bus_format,
+};
+
+static int xlnx_tpg_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct xlnx_tpg *tpg = crtc_to_tpg(crtc);
+
+ xlnx_tpg_enable_irq(tpg);
+
+ return 0;
+}
+
+static void xlnx_tpg_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct xlnx_tpg *tpg = crtc_to_tpg(crtc);
+
+ xlnx_tpg_disable_irq(tpg);
+}
+
+static const struct drm_crtc_funcs xlnx_tpg_crtc_funcs = {
+ .reset = drm_atomic_helper_crtc_reset,
+ .destroy = drm_crtc_cleanup,
+ .set_config = drm_atomic_helper_set_config,
+ .page_flip = drm_atomic_helper_page_flip,
+ .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ .enable_vblank = xlnx_tpg_crtc_enable_vblank,
+ .disable_vblank = xlnx_tpg_crtc_disable_vblank,
+};
+
+/* -----------------------------------------------------------------------------
+ * Setup & Init
+ */
+
+/**
+ * xlnx_tpg_pipeline_init - Initialize DRM pipeline
+ * @drm: DRM device
+ *
+ * Create and link CRTC, plane, and encoder. Attach external DRM bridge.
+ *
+ * Return: 0 on success, or a negative error code otherwise
+ */
+static int xlnx_tpg_pipeline_init(struct drm_device *drm)
+{
+ static const uint32_t xlnx_tpg_formats[] = {
+ DRM_FORMAT_XRGB8888,
+ };
+ static const uint64_t xlnx_tpg_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID,
+ };
+
+ struct xlnx_tpg *tpg = dev_get_drvdata(drm->dev);
+ struct drm_connector *connector;
+ struct drm_encoder *encoder = &tpg->drm->encoder;
+ struct drm_plane *plane = &tpg->drm->plane;
+ struct drm_crtc *crtc = &tpg->drm->crtc;
+ int ret;
+
+ ret = xlnx_tpg_map_resources(tpg);
+ if (ret < 0)
+ return ret;
+
+ ret = xlnx_tpg_setup_irq(tpg);
+ if (ret < 0)
+ return ret;
+
+ drm_plane_helper_add(plane, &xlnx_tpg_plane_helper_funcs);
+ ret = drm_universal_plane_init(drm, plane, 0,
+ &xlnx_tpg_plane_funcs,
+ xlnx_tpg_formats,
+ ARRAY_SIZE(xlnx_tpg_formats),
+ xlnx_tpg_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret) {
+ dev_err(drm->dev, "failed to init plane: %d\n", ret);
+ return ret;
+ }
+
+ drm_crtc_helper_add(crtc, &xlnx_tpg_crtc_helper_funcs);
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL,
+ &xlnx_tpg_crtc_funcs, NULL);
+ if (ret) {
+ dev_err(drm->dev, "failed to init crtc: %d\n", ret);
+ return ret;
+ }
+
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+ ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_NONE);
+ if (ret) {
+ dev_err(drm->dev, "failed to init encoder: %d\n", ret);
+ return ret;
+ }
+
+ ret = drm_bridge_attach(encoder, tpg->disp_bridge, NULL,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (ret < 0) {
+ dev_err(drm->dev, "failed to attach bridge to encoder: %d\n", ret);
+ return ret;
+ }
+
+ connector = drm_bridge_connector_init(drm, encoder);
+ if (IS_ERR(connector)) {
+ ret = PTR_ERR(connector);
+ dev_err(drm->dev, "failed to init connector: %d\n", ret);
+ return ret;
+ }
+
+ ret = drm_connector_attach_encoder(connector, encoder);
+ if (ret < 0) {
+ dev_err(drm->dev, "failed to attach encoder: %d\n", ret);
+ return ret;
+ }
+
+ xlnx_tpg_create_properties(tpg);
+
+ return 0;
+}
+
+static const struct drm_mode_config_funcs xlnx_tpg_mode_config_funcs = {
+ .fb_create = drm_gem_fb_create,
+ .atomic_check = drm_atomic_helper_check,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+DEFINE_DRM_GEM_DMA_FOPS(xlnx_tpg_gem_dma_fops);
+static struct drm_driver xlnx_tpg_drm_driver = {
+ .driver_features = DRIVER_MODESET | DRIVER_GEM |
+ DRIVER_ATOMIC,
+ .fops = &xlnx_tpg_gem_dma_fops,
+ .name = DRIVER_NAME,
+ .desc = DRIVER_DESC,
+ .date = DRIVER_DATE,
+ .major = DRIVER_MAJOR,
+ .minor = DRIVER_MINOR,
+ DRM_GEM_DMA_DRIVER_OPS,
+};
+
+/**
+ * xlnx_tpg_drm_init - Initialize DRM device
+ * @dev: The device
+ *
+ * Allocate and initialize DRM device. Configure mode config and initialize
+ * TPG DRM pipeline.
+ *
+ * Return: 0 on success, or a negative error code otherwise
+ */
+static int xlnx_tpg_drm_init(struct device *dev)
+{
+ struct xlnx_tpg *tpg = dev_get_drvdata(dev);
+ struct drm_device *drm;
+ int ret;
+
+ tpg->drm = devm_drm_dev_alloc(dev, &xlnx_tpg_drm_driver,
+ struct xlnx_tpg_drm, dev);
+ if (IS_ERR(tpg->drm))
+ return PTR_ERR(tpg->drm);
+ tpg->drm->tpg = tpg;
+ drm = &tpg->drm->dev;
+
+ ret = drm_mode_config_init(drm);
+ if (ret < 0)
+ return ret;
+
+ tpg->drm->dev.mode_config.funcs = &xlnx_tpg_mode_config_funcs;
+ tpg->drm->dev.mode_config.min_width = 0;
+ tpg->drm->dev.mode_config.min_height = 0;
+ tpg->drm->dev.mode_config.max_width = 4096;
+ tpg->drm->dev.mode_config.max_height = 4096;
+
+ ret = drm_vblank_init(drm, 1);
+ if (ret < 0)
+ return ret;
+
+ drm_kms_helper_poll_init(drm);
+
+ ret = xlnx_tpg_pipeline_init(drm);
+ if (ret < 0)
+ goto err_poll_fini;
+
+ drm_mode_config_reset(drm);
+
+ ret = drm_dev_register(drm, 0);
+ if (ret < 0)
+ goto err_poll_fini;
+
+ return ret;
+
+err_poll_fini:
+ drm_kms_helper_poll_fini(drm);
+
+ return ret;
+}
+
+/**
+ * xlnx_tpg_drm_fini - Finilize DRM device
+ * @dev: The device
+ */
+static void xlnx_tpg_drm_fini(struct device *dev)
+{
+ struct xlnx_tpg *tpg = dev_get_drvdata(dev);
+
+ drm_kms_helper_poll_fini(&tpg->drm->dev);
+}
+
+static int xlnx_tpg_probe(struct platform_device *pdev)
+{
+ struct xlnx_tpg *tpg;
+ struct device_node *node, *vtc_node;
+ int ret;
+
+ tpg = devm_kzalloc(&pdev->dev, sizeof(*tpg), GFP_KERNEL);
+ if (!tpg)
+ return -ENOMEM;
+
+ tpg->pdev = pdev;
+ platform_set_drvdata(pdev, tpg);
+ node = pdev->dev.of_node;
+
+ tpg->disp_bridge = devm_drm_of_get_bridge(&pdev->dev, node, 0, 0);
+ if (IS_ERR(tpg->disp_bridge)) {
+ ret = PTR_ERR(tpg->disp_bridge);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to discover display bridge\n");
+ return ret;
+ }
+
+ if (of_property_read_u32(node, "bus-format", &tpg->output_bus_format)) {
+ dev_err(&pdev->dev, "required bus-format property undefined\n");
+ return -EINVAL;
+ }
+ tpg->color_format = xlnx_tpg_bus_to_color_format(tpg->output_bus_format);
+
+ vtc_node = of_parse_phandle(node, "xlnx,bridge", 0);
+ if (!vtc_node) {
+ dev_err(&pdev->dev, "required vtc node is missing\n");
+ return -EINVAL;
+ }
+ ret = xlnx_of_find_vtc(vtc_node, &tpg->vtc);
+ if (ret < 0)
+ return ret;
+
+ ret = xlnx_tpg_drm_init(&pdev->dev);
+ if (ret < 0)
+ return ret;
+
+ dev_info(&pdev->dev, "xlnx-tpg driver probed\n");
+
+ return 0;
+}
+
+static int xlnx_tpg_remove(struct platform_device *pdev)
+{
+ xlnx_tpg_drm_fini(&pdev->dev);
+
+ return 0;
+}
+
+static const struct of_device_id xlnx_tpg_of_match[] = {
+ { .compatible = "xlnx,v-tpg-8.2", },
+ { .compatible = "xlnx,v-tpg-8.0", },
+ { /* end of table */ },
+};
+MODULE_DEVICE_TABLE(of, xlnx_tpg_of_match);
+
+static struct platform_driver xlnx_tpg_driver = {
+ .probe = xlnx_tpg_probe,
+ .remove = xlnx_tpg_remove,
+ .driver = {
+ .name = "xlnx-tpg",
+ .of_match_table = xlnx_tpg_of_match,
+ },
+};
+
+module_platform_driver(xlnx_tpg_driver);
+
+MODULE_AUTHOR("Anatoliy Klymenko");
+MODULE_DESCRIPTION("Xilinx TPG CRTC Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/xlnx/xlnx_vtc.c b/drivers/gpu/drm/xlnx/xlnx_vtc.c
new file mode 100644
index 000000000000..351b736706d1
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_vtc.c
@@ -0,0 +1,452 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Video Timing Controller support for Xilinx DRM KMS
+ *
+ * Copyright (C) 2013 - 2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Hyun Woo Kwon <[email protected]>
+ * Saurabh Sengar <[email protected]>
+ * Vishal Sagar <[email protected]>
+ * Anatoliy Klymenko <[email protected]>
+ *
+ * This driver adds support to control the Xilinx Video Timing
+ * Controller connected to the CRTC.
+ */
+
+#include "xlnx_vtc.h"
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <video/videomode.h>
+
+/* register offsets */
+#define XVTC_CTL 0x000
+#define XVTC_VER 0x010
+#define XVTC_GASIZE 0x060
+#define XVTC_GENC 0x068
+#define XVTC_GPOL 0x06c
+#define XVTC_GHSIZE 0x070
+#define XVTC_GVSIZE 0x074
+#define XVTC_GHSYNC 0x078
+#define XVTC_GVBHOFF_F0 0x07c
+#define XVTC_GVSYNC_F0 0x080
+#define XVTC_GVSHOFF_F0 0x084
+#define XVTC_GVBHOFF_F1 0x088
+#define XVTC_GVSYNC_F1 0x08C
+#define XVTC_GVSHOFF_F1 0x090
+#define XVTC_GASIZE_F1 0x094
+
+/* vtc control register bits */
+#define XVTC_CTL_SWRESET BIT(31)
+#define XVTC_CTL_FIPSS BIT(26)
+#define XVTC_CTL_ACPSS BIT(25)
+#define XVTC_CTL_AVPSS BIT(24)
+#define XVTC_CTL_HSPSS BIT(23)
+#define XVTC_CTL_VSPSS BIT(22)
+#define XVTC_CTL_HBPSS BIT(21)
+#define XVTC_CTL_VBPSS BIT(20)
+#define XVTC_CTL_VCSS BIT(18)
+#define XVTC_CTL_VASS BIT(17)
+#define XVTC_CTL_VBSS BIT(16)
+#define XVTC_CTL_VSSS BIT(15)
+#define XVTC_CTL_VFSS BIT(14)
+#define XVTC_CTL_VTSS BIT(13)
+#define XVTC_CTL_HBSS BIT(11)
+#define XVTC_CTL_HSSS BIT(10)
+#define XVTC_CTL_HFSS BIT(9)
+#define XVTC_CTL_HTSS BIT(8)
+#define XVTC_CTL_GE BIT(2)
+#define XVTC_CTL_RU BIT(1)
+
+/* vtc generator polarity register bits */
+#define XVTC_GPOL_FIP BIT(6)
+#define XVTC_GPOL_ACP BIT(5)
+#define XVTC_GPOL_AVP BIT(4)
+#define XVTC_GPOL_HSP BIT(3)
+#define XVTC_GPOL_VSP BIT(2)
+#define XVTC_GPOL_HBP BIT(1)
+#define XVTC_GPOL_VBP BIT(0)
+
+/* vtc generator horizontal 1 */
+#define XVTC_GH1_BPSTART_MASK GENMASK(28, 16)
+#define XVTC_GH1_BPSTART_SHIFT 16
+#define XVTC_GH1_SYNCSTART_MASK GENMASK(12, 0)
+/* vtc generator vertical 1 (field 0) */
+#define XVTC_GV1_BPSTART_MASK GENMASK(28, 16)
+#define XVTC_GV1_BPSTART_SHIFT 16
+#define XVTC_GV1_SYNCSTART_MASK GENMASK(12, 0)
+/* vtc generator/detector vblank/vsync horizontal offset registers */
+#define XVTC_XVXHOX_HEND_MASK GENMASK(28, 16)
+#define XVTC_XVXHOX_HEND_SHIFT 16
+#define XVTC_XVXHOX_HSTART_MASK GENMASK(12, 0)
+
+#define XVTC_GHFRAME_HSIZE GENMASK(12, 0)
+#define XVTC_GVFRAME_HSIZE_F1 GENMASK(12, 0)
+#define XVTC_GA_ACTSIZE_MASK GENMASK(12, 0)
+
+/* vtc generator encoding register bits */
+#define XVTC_GENC_INTERL BIT(6)
+
+/**
+ * struct xlnx_vtc - Xilinx VTC object
+ *
+ * @list: vtc list entry
+ * @dev: device structure
+ * @base: base addr
+ * @ppc: pixels per clock
+ * @axi_clk: AXI Lite clock
+ * @vid_clk: Video clock
+ */
+struct xlnx_vtc {
+ struct xlnx_vtc_iface iface;
+ struct device *dev;
+ void __iomem *base;
+ u32 ppc;
+ struct clk *axi_clk;
+ struct clk *vid_clk;
+};
+
+#define iface_to_vtc(ptr) container_of(ptr, struct xlnx_vtc, iface)
+
+static inline void xlnx_vtc_writel(void __iomem *base, int offset, u32 val)
+{
+ writel(val, base + offset);
+}
+
+static inline u32 xlnx_vtc_readl(void __iomem *base, int offset)
+{
+ return readl(base + offset);
+}
+
+static void xlnx_vtc_reset(struct xlnx_vtc *vtc)
+{
+ u32 reg;
+
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, XVTC_CTL_SWRESET);
+
+ /* enable register update */
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg | XVTC_CTL_RU);
+}
+
+/**
+ * xlnx_vtc_enable - Enable the VTC
+ * @vtc: xilinx vtc structure pointer
+ *
+ * Return:
+ * Zero on success.
+ *
+ * This function enables the VTC
+ */
+static int xlnx_vtc_enable(struct xlnx_vtc_iface *iface)
+{
+ u32 reg;
+ struct xlnx_vtc *vtc = iface_to_vtc(iface);
+
+ /* enable generator */
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg | XVTC_CTL_GE);
+ dev_dbg(vtc->dev, "enabled\n");
+ return 0;
+}
+
+/**
+ * xlnx_vtc_disable - Disable the VTC
+ * @bridge: xilinx vtc structure pointer
+ *
+ * This function disables and resets the VTC.
+ */
+static void xlnx_vtc_disable(struct xlnx_vtc_iface *iface)
+{
+ u32 reg;
+ struct xlnx_vtc *vtc = iface_to_vtc(iface);
+
+ /* disable generator and reset */
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg & ~XVTC_CTL_GE);
+ xlnx_vtc_reset(vtc);
+ dev_dbg(vtc->dev, "disabled\n");
+}
+
+/**
+ * xlnx_vtc_set_timing - Configures the VTC
+ * @vtc: xilinx vtc structure pointer
+ * @vm: video mode requested
+ *
+ * Return:
+ * Zero on success.
+ *
+ * This function calculates the timing values from the video mode
+ * structure passed from the CRTC and configures the VTC.
+ */
+static int xlnx_vtc_set_timing(struct xlnx_vtc_iface *iface,
+ struct videomode *vm)
+{
+ int ret;
+ u32 reg;
+ u32 htotal, hactive, hsync_start, hbackporch_start;
+ u32 vtotal, vactive, vsync_start, vbackporch_start;
+ struct xlnx_vtc *vtc = iface_to_vtc(iface);
+
+ /* Make sure video clock is in sync with video timing */
+ ret = clk_set_rate(vtc->vid_clk, vm->pixelclock / vtc->ppc);
+ if (ret) {
+ dev_err(vtc->dev, "failed to set video clock rate: %d\n", ret);
+ return ret;
+ }
+
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg & ~XVTC_CTL_RU);
+
+ vm->hactive /= vtc->ppc;
+ vm->hfront_porch /= vtc->ppc;
+ vm->hback_porch /= vtc->ppc;
+ vm->hsync_len /= vtc->ppc;
+
+ htotal = vm->hactive + vm->hfront_porch + vm->hsync_len +
+ vm->hback_porch;
+ vtotal = vm->vactive + vm->vfront_porch + vm->vsync_len +
+ vm->vback_porch;
+
+ hactive = vm->hactive;
+ vactive = vm->vactive;
+
+ hsync_start = vm->hactive + vm->hfront_porch;
+ vsync_start = vm->vactive + vm->vfront_porch;
+
+ hbackporch_start = hsync_start + vm->hsync_len;
+ vbackporch_start = vsync_start + vm->vsync_len;
+
+ dev_dbg(vtc->dev, "ha: %d, va: %d\n", hactive, vactive);
+ dev_dbg(vtc->dev, "ht: %d, vt: %d\n", htotal, vtotal);
+ dev_dbg(vtc->dev, "hs: %d, hb: %d\n", hsync_start, hbackporch_start);
+ dev_dbg(vtc->dev, "vs: %d, vb: %d\n", vsync_start, vbackporch_start);
+
+ reg = htotal & XVTC_GHFRAME_HSIZE;
+ xlnx_vtc_writel(vtc->base, XVTC_GHSIZE, reg);
+
+ reg = vtotal & XVTC_GVFRAME_HSIZE_F1;
+ reg |= reg << XVTC_GV1_BPSTART_SHIFT;
+ xlnx_vtc_writel(vtc->base, XVTC_GVSIZE, reg);
+
+ reg = hactive & XVTC_GA_ACTSIZE_MASK;
+ reg |= (vactive & XVTC_GA_ACTSIZE_MASK) << 16;
+ xlnx_vtc_writel(vtc->base, XVTC_GASIZE, reg);
+
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED)
+ xlnx_vtc_writel(vtc->base, XVTC_GASIZE_F1, reg);
+
+ reg = hsync_start & XVTC_GH1_SYNCSTART_MASK;
+ reg |= (hbackporch_start << XVTC_GH1_BPSTART_SHIFT) &
+ XVTC_GH1_BPSTART_MASK;
+ xlnx_vtc_writel(vtc->base, XVTC_GHSYNC, reg);
+
+ reg = vsync_start & XVTC_GV1_SYNCSTART_MASK;
+ reg |= (vbackporch_start << XVTC_GV1_BPSTART_SHIFT) &
+ XVTC_GV1_BPSTART_MASK;
+ xlnx_vtc_writel(vtc->base, XVTC_GVSYNC_F0, reg);
+
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED) {
+ xlnx_vtc_writel(vtc->base, XVTC_GVSYNC_F1, reg);
+ reg = xlnx_vtc_readl(vtc->base, XVTC_GENC) | XVTC_GENC_INTERL;
+ xlnx_vtc_writel(vtc->base, XVTC_GENC, reg);
+ } else {
+ reg = xlnx_vtc_readl(vtc->base, XVTC_GENC) & ~XVTC_GENC_INTERL;
+ xlnx_vtc_writel(vtc->base, XVTC_GENC, reg);
+ }
+
+ /* configure horizontal offset */
+ /* Calculate and update Generator VBlank Hori field 0 */
+ reg = hactive & XVTC_XVXHOX_HSTART_MASK;
+ reg |= (hactive << XVTC_XVXHOX_HEND_SHIFT) &
+ XVTC_XVXHOX_HEND_MASK;
+ xlnx_vtc_writel(vtc->base, XVTC_GVBHOFF_F0, reg);
+
+ /* Calculate and update Generator VSync Hori field 0 */
+ reg = hsync_start & XVTC_XVXHOX_HSTART_MASK;
+ reg |= (hsync_start << XVTC_XVXHOX_HEND_SHIFT) &
+ XVTC_XVXHOX_HEND_MASK;
+ xlnx_vtc_writel(vtc->base, XVTC_GVSHOFF_F0, reg);
+
+ /* Calculate and update Generator VBlank Hori field 1 */
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED) {
+ reg = hactive & XVTC_XVXHOX_HSTART_MASK;
+ reg |= (hactive << XVTC_XVXHOX_HEND_SHIFT) &
+ XVTC_XVXHOX_HEND_MASK;
+ xlnx_vtc_writel(vtc->base, XVTC_GVBHOFF_F1, reg);
+ }
+
+ /* Calculate and update Generator VBlank Hori field 1 */
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED) {
+ reg = (hsync_start - (htotal / 2)) & XVTC_XVXHOX_HSTART_MASK;
+ reg |= ((hsync_start - (htotal / 2)) <<
+ XVTC_XVXHOX_HEND_SHIFT) & XVTC_XVXHOX_HEND_MASK;
+ } else {
+ reg = hsync_start & XVTC_XVXHOX_HSTART_MASK;
+ reg |= (hsync_start << XVTC_XVXHOX_HEND_SHIFT) &
+ XVTC_XVXHOX_HEND_MASK;
+ }
+
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED)
+ xlnx_vtc_writel(vtc->base, XVTC_GVSHOFF_F1, reg);
+
+ /* configure polarity of signals */
+ reg = 0;
+ reg |= XVTC_GPOL_ACP;
+ reg |= XVTC_GPOL_AVP;
+ if (vm->flags & DISPLAY_FLAGS_INTERLACED)
+ reg |= XVTC_GPOL_FIP;
+ if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH) {
+ reg |= XVTC_GPOL_VBP;
+ reg |= XVTC_GPOL_VSP;
+ }
+ if (vm->flags & DISPLAY_FLAGS_HSYNC_HIGH) {
+ reg |= XVTC_GPOL_HBP;
+ reg |= XVTC_GPOL_HSP;
+ }
+ xlnx_vtc_writel(vtc->base, XVTC_GPOL, reg);
+
+ /* configure timing source */
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ reg |= XVTC_CTL_VCSS;
+ reg |= XVTC_CTL_VASS;
+ reg |= XVTC_CTL_VBSS;
+ reg |= XVTC_CTL_VSSS;
+ reg |= XVTC_CTL_VFSS;
+ reg |= XVTC_CTL_VTSS;
+ reg |= XVTC_CTL_HBSS;
+ reg |= XVTC_CTL_HSSS;
+ reg |= XVTC_CTL_HFSS;
+ reg |= XVTC_CTL_HTSS;
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg);
+
+ reg = xlnx_vtc_readl(vtc->base, XVTC_CTL);
+ xlnx_vtc_writel(vtc->base, XVTC_CTL, reg | XVTC_CTL_RU);
+ dev_dbg(vtc->dev, "set timing done\n");
+
+ return 0;
+}
+
+static int xlnx_vtc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct xlnx_vtc *vtc;
+ struct resource *res;
+ int ret;
+
+ vtc = devm_kzalloc(dev, sizeof(*vtc), GFP_KERNEL);
+ if (!vtc)
+ return -ENOMEM;
+
+ vtc->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "failed to get resource for device\n");
+ return -EFAULT;
+ }
+
+ vtc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(vtc->base)) {
+ dev_err(dev, "failed to remap io region\n");
+ return PTR_ERR(vtc->base);
+ }
+
+ platform_set_drvdata(pdev, vtc);
+
+ ret = of_property_read_u32(dev->of_node, "xlnx,pixels-per-clock",
+ &vtc->ppc);
+ if (ret || (vtc->ppc != 1 && vtc->ppc != 2 && vtc->ppc != 4)) {
+ dev_err(dev, "failed to get ppc\n");
+ return ret;
+ }
+ dev_info(dev, "vtc ppc = %d\n", vtc->ppc);
+
+ vtc->axi_clk = devm_clk_get(vtc->dev, "s_axi_aclk");
+ if (IS_ERR(vtc->axi_clk)) {
+ ret = PTR_ERR(vtc->axi_clk);
+ dev_err(dev, "failed to get axi lite clk %d\n", ret);
+ return ret;
+ }
+
+ vtc->vid_clk = devm_clk_get(vtc->dev, "clk");
+ if (IS_ERR(vtc->vid_clk)) {
+ ret = PTR_ERR(vtc->vid_clk);
+ dev_err(dev, "failed to get video clk %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(vtc->axi_clk);
+ if (ret) {
+ dev_err(vtc->dev, "unable to enable axilite clk %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(vtc->vid_clk);
+ if (ret) {
+ dev_err(vtc->dev, "unable to enable video clk %d\n", ret);
+ goto err_axi_clk;
+ }
+
+ xlnx_vtc_reset(vtc);
+
+ vtc->iface.of_node = dev->of_node;
+ vtc->iface.enable = xlnx_vtc_enable;
+ vtc->iface.disable = xlnx_vtc_disable;
+ vtc->iface.set_timing = xlnx_vtc_set_timing;
+
+ xlnx_vtc_register(&vtc->iface);
+
+ dev_info(dev, "Xilinx VTC IP version : 0x%08x\n",
+ xlnx_vtc_readl(vtc->base, XVTC_VER));
+ dev_info(dev, "Xilinx VTC DRM Bridge driver probed\n");
+ return 0;
+
+err_axi_clk:
+ clk_disable_unprepare(vtc->axi_clk);
+ return ret;
+}
+
+static int xlnx_vtc_remove(struct platform_device *pdev)
+{
+ struct xlnx_vtc *vtc = platform_get_drvdata(pdev);
+
+ xlnx_vtc_unregister(&vtc->iface);
+
+ clk_disable_unprepare(vtc->vid_clk);
+ clk_disable_unprepare(vtc->axi_clk);
+
+ return 0;
+}
+
+static const struct of_device_id xlnx_vtc_of_match[] = {
+ { .compatible = "xlnx,bridge-v-tc-6.1" },
+ { /* end of table */ },
+};
+
+MODULE_DEVICE_TABLE(of, xlnx_vtc_of_match);
+
+static struct platform_driver xlnx_vtc_bridge_driver = {
+ .probe = xlnx_vtc_probe,
+ .remove = xlnx_vtc_remove,
+ .driver = {
+ .name = "xlnx,bridge-vtc",
+ .of_match_table = xlnx_vtc_of_match,
+ },
+};
+
+module_init(xlnx_vtc_list_init);
+module_platform_driver(xlnx_vtc_bridge_driver);
+module_exit(xlnx_vtc_list_fini);
+
+MODULE_AUTHOR("Vishal Sagar");
+MODULE_DESCRIPTION("Xilinx VTC Bridge Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/xlnx/xlnx_vtc.h b/drivers/gpu/drm/xlnx/xlnx_vtc.h
new file mode 100644
index 000000000000..8abe01a5d943
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_vtc.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Xilinx DRM VTC header
+ *
+ * Copyright (C) 2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Anatoliy Klymenko <[email protected]>
+ */
+
+#ifndef _XLNX_VTC_H_
+#define _XLNX_VTC_H_
+
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+#include <linux/of.h>
+#include <linux/types.h>
+#include <video/videomode.h>
+
+/**
+ * struct xlnx_vtc_iface - Video Timing Controller interface
+ * @list: VTC list entry
+ * @of_node: Device tree node
+ * @enable: Enable VTC callback
+ * @disable: Disable VTC callback
+ * @set_timing: Program VTC timing callback
+ */
+struct xlnx_vtc_iface {
+ struct list_head list;
+ struct device_node *of_node;
+ int (*enable)(struct xlnx_vtc_iface *vtc);
+ void (*disable)(struct xlnx_vtc_iface *vtc);
+ int (*set_timing)(struct xlnx_vtc_iface *vtc, struct videomode *vm);
+};
+
+#if IS_ENABLED(CONFIG_DRM_XLNX_BRIDGE_VTC)
+
+int xlnx_vtc_iface_enable(struct xlnx_vtc_iface *vtc);
+void xlnx_vtc_iface_disable(struct xlnx_vtc_iface *vtc);
+int xlnx_vtc_iface_set_timing(struct xlnx_vtc_iface *vtc,
+ struct videomode *vm);
+
+int xlnx_vtc_list_init(void) __init;
+void xlnx_vtc_list_fini(void) __exit;
+
+int xlnx_vtc_register(struct xlnx_vtc_iface *vtc);
+void xlnx_vtc_unregister(struct xlnx_vtc_iface *vtc);
+int xlnx_of_find_vtc(const struct device_node *np,
+ struct xlnx_vtc_iface **vtc);
+
+#else /* CONFIG_DRM_XLNX_BRIDGE_VTC */
+
+static inline int xlnx_vtc_iface_enable(struct xlnx_vtc_iface *vtc)
+{
+ return vtc ? -ENODEV : 0;
+}
+
+static inline xlnx_vtc_iface_disable(struct xlnx_vtc_iface *vtc)
+{
+}
+
+static inline int xlnx_vtc_iface_set_timing(struct xlnx_vtc_iface *vtc,
+ struct videomode *vm)
+{
+ return vtc ? -ENODEV : 0;
+}
+
+static inline int xlnx_of_find_vtc(const struct device_node *np,
+ struct xlnx_vtc_iface **vtc)
+{
+ *vtc = NULL;
+ return -ENODEV;
+}
+
+static inline int xlnx_vtc_list_init(void)
+{
+ return 0;
+}
+
+static inline void xlnx_vtc_list_fini(void)
+{
+}
+
+static inline int xlnx_vtc_register(struct xlnx_vtc_iface *vtc)
+{
+ return 0;
+}
+
+static inline void xlnx_vtc_unregister(struct xlnx_vtc_iface *vtc)
+{
+}
+
+static inline int xlnx_of_find_vtc(const struct device_node *np,
+ struct xlnx_vtc_iface **vtc)
+{
+ *vtc = NULL;
+ return -ENODEV;
+}
+
+#endif /* CONFIG_DRM_XLNX_BRIDGE_VTC */
+
+#endif /* _XLNX_VTC_H_ */
diff --git a/drivers/gpu/drm/xlnx/xlnx_vtc_list.c b/drivers/gpu/drm/xlnx/xlnx_vtc_list.c
new file mode 100644
index 000000000000..fe8d8447a18c
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_vtc_list.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Video Timing Controller List
+ *
+ * Copyright (C) 2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Anatoliy Klymenko <[email protected]>
+ */
+
+#include "xlnx_vtc.h"
+
+#include <linux/mutex.h>
+
+/**
+ * struct xlnx_vtc_list - List of registered Video Timing Controllers
+ * @head: Head of the list of registered VTC instances
+ * @lock: Mutex protecting the list
+ * @initialized: Initialization flag
+ */
+struct xlnx_vtc_list {
+ struct list_head head;
+ struct mutex lock;
+ bool initialized;
+};
+
+static struct xlnx_vtc_list vtc_list;
+
+/**
+ * xlnx_vtc_list_init - Initialize VTC list
+ *
+ * Return 0 on success, or error code otherwise
+ */
+int xlnx_vtc_list_init(void)
+{
+ if (!vtc_list.initialized) {
+ INIT_LIST_HEAD(&vtc_list.head);
+ mutex_init(&vtc_list.lock);
+ vtc_list.initialized = true;
+ }
+
+ return 0;
+}
+
+/**
+ * xlnx_vtc_list_fini - Deinitialize VTC list, free resources
+ */
+void xlnx_vtc_list_fini(void)
+{
+ if (vtc_list.initialized) {
+ mutex_destroy(&vtc_list.lock);
+ vtc_list.initialized = false;
+ }
+}
+
+/**
+ * xlnx_vtc_register - Register new VTC instance
+ * @vtc: Pointer to VTC interface instance to register
+ *
+ * Return 0 on success, or error code otherwise
+ */
+int xlnx_vtc_register(struct xlnx_vtc_iface *vtc)
+{
+ if (!vtc || !vtc->of_node)
+ return -EINVAL;
+
+ if (!vtc_list.initialized)
+ return -EFAULT;
+
+ mutex_lock(&vtc_list.lock);
+ list_add_tail(&vtc->list, &vtc_list.head);
+ mutex_unlock(&vtc_list.lock);
+
+ return 0;
+}
+
+/**
+ * xlnx_vtc_unregister - Register new VTC instance
+ * @vtc: The VTC interface instance
+ */
+void xlnx_vtc_unregister(struct xlnx_vtc_iface *vtc)
+{
+ if (!vtc || !vtc_list.initialized)
+ return;
+
+ mutex_lock(&vtc_list.lock);
+ list_del(&vtc->list);
+ mutex_unlock(&vtc_list.lock);
+}
+
+/**
+ * xlnx_of_find_vtc - Lookup VTC instance by OF node pointer
+ * @np: Pointer to VTC device node
+ * @vtc: Output vtc instance pointer
+ *
+ * Return 0 on success, or error code otherwise
+ */
+int xlnx_of_find_vtc(const struct device_node *np, struct xlnx_vtc_iface **vtc)
+{
+ struct xlnx_vtc_iface *vtc_pos;
+ int ret = -EPROBE_DEFER;
+
+ *vtc = NULL;
+
+ if (!vtc_list.initialized)
+ return ret;
+
+ mutex_lock(&vtc_list.lock);
+ list_for_each_entry(vtc_pos, &vtc_list.head, list) {
+ if (vtc_pos->of_node == np) {
+ *vtc = vtc_pos;
+ ret = 0;
+ break;
+ }
+ }
+ mutex_unlock(&vtc_list.lock);
+
+ return ret;
+}
+
+/**
+ * xlnx_vtc_iface_enable - Enable VTC
+ * @vtc: The VTC
+ *
+ * Return 0 on success, or error code otherwise
+ */
+int xlnx_vtc_iface_enable(struct xlnx_vtc_iface *vtc)
+{
+ if (!vtc || !vtc->enable)
+ return -EINVAL;
+
+ return vtc->enable(vtc);
+}
+
+/**
+ * xlnx_vtc_iface_disable - Disable VTC
+ * @vtc: The VTC
+ */
+void xlnx_vtc_iface_disable(struct xlnx_vtc_iface *vtc)
+{
+ if (!vtc || !vtc->disable)
+ return;
+
+ vtc->disable(vtc);
+}
+
+/**
+ * xlnx_vtc_iface_set_timing - Program VTC video timing
+ * @vtc: The VTC
+ * @vm: Video mode to program timing for
+ *
+ * Return 0 on success, or error code otherwise
+ */
+int xlnx_vtc_iface_set_timing(struct xlnx_vtc_iface *vtc,
+ struct videomode *vm)
+{
+ if (!vtc || !vtc->set_timing)
+ return -EINVAL;
+
+ return vtc->set_timing(vtc, vm);
+}
--
2.25.1
On Thu, 21 Mar 2024 13:43:46 -0700, Anatoliy Klymenko wrote:
> DO NOT MERGE. REFERENCE ONLY.
>
> Add binding for AMD/Xilinx Video Timing Controller and Test Pattern
> Generator.
>
> Copy media-bus-formats.h into dt-bindings/media to suplement TPG DT node.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> .../bindings/display/xlnx/xlnx,v-tpg.yaml | 87 ++++++++++
> .../devicetree/bindings/display/xlnx/xlnx,vtc.yaml | 65 ++++++++
> include/dt-bindings/media/media-bus-format.h | 177 +++++++++++++++++++++
> 3 files changed, 329 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml:35:4: [warning] wrong indentation: expected 4 but found 3 (indentation)
/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml:45:8: [warning] wrong indentation: expected 8 but found 7 (indentation)
/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml:49:8: [warning] wrong indentation: expected 8 but found 7 (indentation)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml: bus-format: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/xlnx/xlnx,v-tpg.yaml: xlnx,bridge: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/xlnx/xlnx,vtc.yaml: xlnx,pixels-per-clock: missing type definition
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> DO NOT MERGE. REFERENCE ONLY.
Why? What are you doing here and why nothing about this is explained?
>
> Add binding for AMD/Xilinx Video Timing Controller and Test Pattern
> Generator.
>
> Copy media-bus-formats.h into dt-bindings/media to suplement TPG DT node.
Still not tested. Do not send untested code to the lists.
NAK
Best regards,
Krzysztof
On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
> new file mode 100644
> index 000000000000..60fc6e11dabc
> --- /dev/null
> +++ b/include/dt-bindings/media/media-bus-format.h
> @@ -0,0 +1,177 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Media Bus API header
> + *
> + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
That's not true. Your SPDX tells something entirely different.
Anyway, you did not explain why you need to copy anything anywhere.
Specifically, random hex values *are not bindings*.
Best regards,
Krzysztof
On Thu, Mar 21, 2024 at 01:43:45PM -0700, Anatoliy Klymenko wrote:
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 2dafc39a27cb..f2e12a3c4e5f 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct drm_device *dev)
> return ret;
> }
> EXPORT_SYMBOL(drm_helper_force_disable_all);
> +
> +/**
> + * drm_helper_crtc_select_output_bus_format - Select output media bus format
> + * @crtc: The CRTC to query
> + * @crtc_state: The new CRTC state
> + * @supported_fmts: List of media bus format options to pick from
> + * @num_supported_fmts: Number of media bus formats in @supported_fmts list
> + *
> + * Encoder drivers may call this helper to give the connected CRTC a chance to
> + * select compatible or preffered media bus format to use over the CRTC encoder
> + * link. Encoders should provide list of supported input MEDIA_BUS_FMT_* for
> + * CRTC to pick from. CRTC driver is expected to select preferred media bus
> + * format from the list and, once enabled, generate the signal accordingly.
> + *
> + * Returns:
> + * Selected preferred media bus format or 0 if CRTC does not support any from
> + * @supported_fmts list.
> + */
> +u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_state,
> + const u32 *supported_fmts,
> + unsigned int num_supported_fmts)
> +{
> + if (!crtc || !supported_fmts || !num_supported_fmts)
> + return 0;
> +
> + if (!crtc->helper_private ||
> + !crtc->helper_private->select_output_bus_format)
> + return supported_fmts[0];
> +
> + return crtc->helper_private->select_output_bus_format(crtc,
> + crtc_state,
> + supported_fmts,
> + num_supported_fmts);
> +}
Again, the output of that selection must be found in the CRTC state,
otherwise CRTCs have no way to know which have been selected.
Maxime
On Fri, Mar 22, 2024 at 06:59:18AM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
> > new file mode 100644
> > index 000000000000..60fc6e11dabc
> > --- /dev/null
> > +++ b/include/dt-bindings/media/media-bus-format.h
> > @@ -0,0 +1,177 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +/*
> > + * Media Bus API header
> > + *
> > + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
>
> That's not true. Your SPDX tells something entirely different.
>
> Anyway, you did not explain why you need to copy anything anywhere.
I assume by "copy anything anywhere" you mean "why did you copy a linux
uapi header into the bindings?
> Specifically, random hex values *are not bindings*.
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,
Thanks a lot for the review.
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Thursday, March 21, 2024 10:59 PM
> To: Klymenko, Anatoliy <[email protected]>; Laurent Pinchart
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard <[email protected]>;
> Thomas Zimmermann <[email protected]>; David Airlie
> <[email protected]>; Daniel Vetter <[email protected]>; Simek, Michal
> <[email protected]>; Andrzej Hajda <[email protected]>; Neil
> Armstrong <[email protected]>; Robert Foss <[email protected]>; Jonas
> Karlman <[email protected]>; Jernej Skrabec <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Mauro Carvalho Chehab <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-
> bindings/media/media-bus-format.h
> > new file mode 100644
> > index 000000000000..60fc6e11dabc
> > --- /dev/null
> > +++ b/include/dt-bindings/media/media-bus-format.h
> > @@ -0,0 +1,177 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > +/*
> > + * Media Bus API header
> > + *
> > + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
>
> That's not true. Your SPDX tells something entirely different.
>
Thank you - I'll see how to fix it.
> Anyway, you did not explain why you need to copy anything anywhere.
>
> Specifically, random hex values *are not bindings*.
>
The same media bus format values are being used by the reference driver in patch #9. And, as far as I know, we cannot use headers from Linux API headers directly (at least I noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance). What would be the best approach to reusing the same defines on DT and driver sides from your point of view? Symlink maybe?
> Best regards,
> Krzysztof
Thank you,
Anatoliy
Hi Maxime,
Thank you for the review.
> -----Original Message-----
> From: Maxime Ripard <[email protected]>
> Sent: Friday, March 22, 2024 2:45 AM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: Laurent Pinchart <[email protected]>; Maarten Lankhorst
> <[email protected]>; Thomas Zimmermann
> <[email protected]>; David Airlie <[email protected]>; Daniel Vetter
> <[email protected]>; Simek, Michal <[email protected]>; Andrzej Hajda
> <[email protected]>; Neil Armstrong <[email protected]>; Robert
> Foss <[email protected]>; Jonas Karlman <[email protected]>; Jernej Skrabec
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Mauro Carvalho Chehab <[email protected]>; Tomi
> Valkeinen <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 7/9] drm/atomic-helper: Add select_output_bus_format
> callback
>
> On Thu, Mar 21, 2024 at 01:43:45PM -0700, Anatoliy Klymenko wrote:
> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> > b/drivers/gpu/drm/drm_crtc_helper.c
> > index 2dafc39a27cb..f2e12a3c4e5f 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -1055,3 +1055,39 @@ int drm_helper_force_disable_all(struct
> drm_device *dev)
> > return ret;
> > }
> > EXPORT_SYMBOL(drm_helper_force_disable_all);
> > +
> > +/**
> > + * drm_helper_crtc_select_output_bus_format - Select output media bus
> > +format
> > + * @crtc: The CRTC to query
> > + * @crtc_state: The new CRTC state
> > + * @supported_fmts: List of media bus format options to pick from
> > + * @num_supported_fmts: Number of media bus formats in
> > +@supported_fmts list
> > + *
> > + * Encoder drivers may call this helper to give the connected CRTC a
> > +chance to
> > + * select compatible or preffered media bus format to use over the
> > +CRTC encoder
> > + * link. Encoders should provide list of supported input
> > +MEDIA_BUS_FMT_* for
> > + * CRTC to pick from. CRTC driver is expected to select preferred
> > +media bus
> > + * format from the list and, once enabled, generate the signal accordingly.
> > + *
> > + * Returns:
> > + * Selected preferred media bus format or 0 if CRTC does not support
> > +any from
> > + * @supported_fmts list.
> > + */
> > +u32 drm_helper_crtc_select_output_bus_format(struct drm_crtc *crtc,
> > + struct drm_crtc_state *crtc_state,
> > + const u32 *supported_fmts,
> > + unsigned int num_supported_fmts) {
> > + if (!crtc || !supported_fmts || !num_supported_fmts)
> > + return 0;
> > +
> > + if (!crtc->helper_private ||
> > + !crtc->helper_private->select_output_bus_format)
> > + return supported_fmts[0];
> > +
> > + return crtc->helper_private->select_output_bus_format(crtc,
> > + crtc_state,
> > + supported_fmts,
> > + num_supported_fmts);
> > +}
>
> Again, the output of that selection must be found in the CRTC state, otherwise
> CRTCs have no way to know which have been selected.
>
Yes, now I got it - thank you. I'll fix this in the next version.
> Maxime
Thank you,
Anatoliy
On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> Implement live video input format setting for ZynqMP DPSUB.
>
> ZynqMP DPSUB can operate in 2 modes: DMA-based and live.
>
> In the live mode, DPSUB receives a live video signal from FPGA-based CRTC.
> DPSUB acts as a DRM encoder bridge in such a scenario. To properly tune
> into the incoming video signal, DPSUB should be programmed with the proper
> media bus format. This patch series addresses this task.
>
> Patch 1/9: Set the DPSUB layer mode of operation prior to enabling the
> layer. Allows to use layer operational mode before its enablement.
>
> Patch 2/9: Update some IP register defines.
>
> Patch 3/9: Factor out some code into a helper function.
>
> Patch 4/9: Announce supported input media bus formats via
> drm_bridge_funcs.atomic_get_input_bus_fmts callback.
>
> Patch 5/9: Minimize usage of a global flag. Minor improvement.
>
> Patch 6/9: Program DPSUB live video input format based on selected bus
> config in the new atomic bridge state.
>
> Patch 7/9: New optional CRTC atomic helper proposal that will allow CRTC
> to participate in DRM bridge chain format negotiation and impose format
> restrictions. Incorporate this callback into the DRM bridge format
> negotiation process.
>
> Patch 8/9: DT bindings documentation for Video Timing Controller and Test
> Pattern Generator IPs.
>
> Patch 9/9: Reference FPGA CRTC driver based on AMD/Xilinx Test Pattern
> Generator (TPG) IP. Add driver for the AMD/Xilinx Video Timing Controller
> (VTC), which supplements TPG.
None of the last users of your API can be merged, therefore this API
should be considered as without users. We do not add API which does not
have any in-tree users.
Best regards,
Krzysztof
On 22/03/2024 20:12, Klymenko, Anatoliy wrote:
> Hi Krzysztof,
>
> Thanks a lot for the review.
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: Thursday, March 21, 2024 10:59 PM
>> To: Klymenko, Anatoliy <[email protected]>; Laurent Pinchart
>> <[email protected]>; Maarten Lankhorst
>> <[email protected]>; Maxime Ripard <[email protected]>;
>> Thomas Zimmermann <[email protected]>; David Airlie
>> <[email protected]>; Daniel Vetter <[email protected]>; Simek, Michal
>> <[email protected]>; Andrzej Hajda <[email protected]>; Neil
>> Armstrong <[email protected]>; Robert Foss <[email protected]>; Jonas
>> Karlman <[email protected]>; Jernej Skrabec <[email protected]>; Rob
>> Herring <[email protected]>; Krzysztof Kozlowski
>> <[email protected]>; Conor Dooley <[email protected]>;
>> Mauro Carvalho Chehab <[email protected]>
>> Cc: Tomi Valkeinen <[email protected]>; dri-
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; linux-
>> [email protected]
>> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
>>
>> Caution: This message originated from an External Source. Use proper caution
>> when opening attachments, clicking links, or responding.
>>
>>
>> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
>>> diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-
>> bindings/media/media-bus-format.h
>>> new file mode 100644
>>> index 000000000000..60fc6e11dabc
>>> --- /dev/null
>>> +++ b/include/dt-bindings/media/media-bus-format.h
>>> @@ -0,0 +1,177 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>>> +/*
>>> + * Media Bus API header
>>> + *
>>> + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>
>> That's not true. Your SPDX tells something entirely different.
>>
>
> Thank you - I'll see how to fix it.
>
>> Anyway, you did not explain why you need to copy anything anywhere.
>>
>> Specifically, random hex values *are not bindings*.
>>
>
> The same media bus format values are being used by the reference driver in patch #9. And, as far as I know, we cannot use headers from Linux API headers directly (at least I
I don't understand what does it mean. You can use in your driver
whatever headers you wish, I don't care about them.
noticed the same pattern in ../dt-bindings/sdtv-standarts.h for
instance). What would be the best approach to reusing the same defines
on DT and driver sides from your point of view? Symlink maybe?
>
Wrap your messages to match mailing list discussion style. There are no
defines used in DT. If there are, show me them in *THIS* or other
*upstreamed* (being upstreamed) patchset.
Whatever you have out of tree or "DO NOT MERGE" does not matter and does
not justify anything.
Best regards,
Krzysztof
On 22/03/2024 19:05, Conor Dooley wrote:
> On Fri, Mar 22, 2024 at 06:59:18AM +0100, Krzysztof Kozlowski wrote:
>> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
>>> diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
>>> new file mode 100644
>>> index 000000000000..60fc6e11dabc
>>> --- /dev/null
>>> +++ b/include/dt-bindings/media/media-bus-format.h
>>> @@ -0,0 +1,177 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>>> +/*
>>> + * Media Bus API header
>>> + *
>>> + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>
>> That's not true. Your SPDX tells something entirely different.
>>
>> Anyway, you did not explain why you need to copy anything anywhere.
>
> I assume by "copy anything anywhere" you mean "why did you copy a linux
> uapi header into the bindings?
Yes, I trimmed context too much.
The reasoning of copying some UAPI and claiming it is a binding was:
"Copy media-bus-formats.h into dt-bindings/media to suplement TPG DT node."
so as seen *there is no reason*.
Commit msg should explain why we are doing things.
Best regards,
Krzysztof
On Sat, Mar 23, 2024 at 11:22:22AM +0100, Krzysztof Kozlowski wrote:
> On 22/03/2024 19:05, Conor Dooley wrote:
> > On Fri, Mar 22, 2024 at 06:59:18AM +0100, Krzysztof Kozlowski wrote:
> >> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> >>> diff --git a/include/dt-bindings/media/media-bus-format.h b/include/dt-bindings/media/media-bus-format.h
> >>> new file mode 100644
> >>> index 000000000000..60fc6e11dabc
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/media/media-bus-format.h
> >>> @@ -0,0 +1,177 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> >>> +/*
> >>> + * Media Bus API header
> >>> + *
> >>> + * Copyright (C) 2009, Guennadi Liakhovetski <[email protected]>
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or modify
> >>> + * it under the terms of the GNU General Public License version 2 as
> >>> + * published by the Free Software Foundation.
> >>
> >> That's not true. Your SPDX tells something entirely different.
> >>
> >> Anyway, you did not explain why you need to copy anything anywhere.
> >
> > I assume by "copy anything anywhere" you mean "why did you copy a linux
> > uapi header into the bindings?
>
> Yes, I trimmed context too much.
>
> The reasoning of copying some UAPI and claiming it is a binding was:
> "Copy media-bus-formats.h into dt-bindings/media to suplement TPG DT node."
> so as seen *there is no reason*.
>
> Commit msg should explain why we are doing things.
Oh for sure. I was just wondering if you were complaining about the UAPI
header or if that comment was about the copyright notice. If it had been
the latter I was gonna point out the former :)
Hi Krzysztof,
Thank you for the feedback.
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Saturday, March 23, 2024 3:21 AM
> To: Klymenko, Anatoliy <[email protected]>; Laurent Pinchart
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard
> <[email protected]>; Thomas Zimmermann <[email protected]>;
> David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Simek,
> Michal <[email protected]>; Andrzej Hajda
> <[email protected]>; Neil Armstrong <[email protected]>;
> Robert Foss <[email protected]>; Jonas Karlman <[email protected]>; Jernej
> Skrabec <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Mauro Carvalho Chehab <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 22/03/2024 20:12, Klymenko, Anatoliy wrote:
> > Hi Krzysztof,
> >
> > Thanks a lot for the review.
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <[email protected]>
> >> Sent: Thursday, March 21, 2024 10:59 PM
> >> To: Klymenko, Anatoliy <[email protected]>; Laurent Pinchart
> >> <[email protected]>; Maarten Lankhorst
> >> <[email protected]>; Maxime Ripard
> >> <[email protected]>; Thomas Zimmermann <[email protected]>;
> David
> >> Airlie <[email protected]>; Daniel Vetter <[email protected]>; Simek,
> >> Michal <[email protected]>; Andrzej Hajda
> >> <[email protected]>; Neil Armstrong
> >> <[email protected]>; Robert Foss <[email protected]>; Jonas
> >> Karlman <[email protected]>; Jernej Skrabec
> <[email protected]>;
> >> Rob Herring <[email protected]>; Krzysztof Kozlowski
> >> <[email protected]>; Conor Dooley
> >> <[email protected]>; Mauro Carvalho Chehab <[email protected]>
> >> Cc: Tomi Valkeinen <[email protected]>; dri-
> >> [email protected]; [email protected];
> >> linux- [email protected]; [email protected]; linux-
> >> [email protected]
> >> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG
> >> bindings
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> >>> diff --git a/include/dt-bindings/media/media-bus-format.h
> >>> b/include/dt-
> >> bindings/media/media-bus-format.h
> >>> new file mode 100644
> >>> index 000000000000..60fc6e11dabc
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/media/media-bus-format.h
> >>> @@ -0,0 +1,177 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> >>> +/*
> >>> + * Media Bus API header
> >>> + *
> >>> + * Copyright (C) 2009, Guennadi Liakhovetski
> >>> +<[email protected]>
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or
> >>> +modify
> >>> + * it under the terms of the GNU General Public License version 2
> >>> +as
> >>> + * published by the Free Software Foundation.
> >>
> >> That's not true. Your SPDX tells something entirely different.
> >>
> >
> > Thank you - I'll see how to fix it.
> >
> >> Anyway, you did not explain why you need to copy anything anywhere.
> >>
> >> Specifically, random hex values *are not bindings*.
> >>
> >
> > The same media bus format values are being used by the reference
> > driver in patch #9. And, as far as I know, we cannot use headers from
> > Linux API headers directly (at least I
>
> I don't understand what does it mean. You can use in your driver whatever
> headers you wish, I don't care about them.
>
>
> noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance).
> What would be the best approach to reusing the same defines on DT and
> driver sides from your point of view? Symlink maybe?
> >
>
> Wrap your messages to match mailing list discussion style. There are no
> defines used in DT. If there are, show me them in *THIS* or other
> *upstreamed* (being upstreamed) patchset.
>
Sorry, I didn't explain properly what I'm trying to achieve. I need to
create a DT node property that represents video signal format, one of
MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would be
nice to reuse the same symbolic values in the device tree. What is the
best approach here? Should I create a separate header in
include/dt-bindings with the same or similar (to avoid multiple
definition errors) defines, or is it better to create a symlink to
media-bus-format.h like include/dt-bindings/linux-event-codes.h?
> Whatever you have out of tree or "DO NOT MERGE" does not matter and
> does not justify anything.
>
>
> Best regards,
> Krzysztof
Thank you,
Anatoliy
On Fri, Mar 29, 2024 at 12:38:33AM +0000, Klymenko, Anatoliy wrote:
> Thank you for the feedback.
> > From: Krzysztof Kozlowski <[email protected]>
> > Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
> > On 22/03/2024 20:12, Klymenko, Anatoliy wrote:
> > >> From: Krzysztof Kozlowski <[email protected]>
> > >> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > >>> diff --git a/include/dt-bindings/media/media-bus-format.h
> > >>> b/include/dt-
> > >> bindings/media/media-bus-format.h
> > >>> new file mode 100644
> > >>> index 000000000000..60fc6e11dabc
> > >>> --- /dev/null
> > >>> +++ b/include/dt-bindings/media/media-bus-format.h
> > >>> @@ -0,0 +1,177 @@
> > >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > >>> +/*
> > >>> + * Media Bus API header
> > >>> + *
> > >>> + * Copyright (C) 2009, Guennadi Liakhovetski
> > >>> +<[email protected]>
> > >>> + *
> > >>> + * This program is free software; you can redistribute it and/or
> > >>> +modify
> > >>> + * it under the terms of the GNU General Public License version 2
> > >>> +as
> > >>> + * published by the Free Software Foundation.
> > >>
> > >> That's not true. Your SPDX tells something entirely different.
> > >>
> > >
> > > Thank you - I'll see how to fix it.
> > >
> > >> Anyway, you did not explain why you need to copy anything anywhere.
> > >>
> > >> Specifically, random hex values *are not bindings*.
> > >>
> > >
> > > The same media bus format values are being used by the reference
> > > driver in patch #9. And, as far as I know, we cannot use headers from
> > > Linux API headers directly (at least I
> >
> > I don't understand what does it mean. You can use in your driver whatever
> > headers you wish, I don't care about them.
> >
> >
> > noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance).
> > What would be the best approach to reusing the same defines on DT and
> > driver sides from your point of view? Symlink maybe?
> > >
> >
> > Wrap your messages to match mailing list discussion style. There are no
> > defines used in DT. If there are, show me them in *THIS* or other
> > *upstreamed* (being upstreamed) patchset.
> >
>
> Sorry, I didn't explain properly what I'm trying to achieve. I need to
> create a DT node property that represents video signal format, one of
> MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would be
> nice to reuse the same symbolic values in the device tree. What is the
> best approach here? Should I create a separate header in
> include/dt-bindings with the same or similar (to avoid multiple
> definition errors) defines, or is it better to create a symlink to
> media-bus-format.h like include/dt-bindings/linux-event-codes.h?
Isn't there already a property for this, described in
Documentation/devicetree/bindings/media/xilinx/video.txt
?
> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Friday, March 29, 2024 8:47 AM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>; Laurent Pinchart
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard
> <[email protected]>; Thomas Zimmermann <[email protected]>;
> David Airlie <[email protected]>; Daniel Vetter <[email protected]>; Simek,
> Michal <[email protected]>; Andrzej Hajda
> <[email protected]>; Neil Armstrong <[email protected]>;
> Robert Foss <[email protected]>; Jonas Karlman <[email protected]>; Jernej
> Skrabec <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Mauro Carvalho Chehab <[email protected]>;
> Tomi Valkeinen <[email protected]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
>
> On Fri, Mar 29, 2024 at 12:38:33AM +0000, Klymenko, Anatoliy wrote:
> > Thank you for the feedback.
> > > From: Krzysztof Kozlowski <[email protected]>
> > > Subject: Re: [PATCH v3 8/9] dt-bindings: xlnx: Add VTC and TPG bindings
> > > On 22/03/2024 20:12, Klymenko, Anatoliy wrote:
> > > >> From: Krzysztof Kozlowski <[email protected]>
> > > >> On 21/03/2024 21:43, Anatoliy Klymenko wrote:
> > > >>> diff --git a/include/dt-bindings/media/media-bus-format.h
> > > >>> b/include/dt-
> > > >> bindings/media/media-bus-format.h
> > > >>> new file mode 100644
> > > >>> index 000000000000..60fc6e11dabc
> > > >>> --- /dev/null
> > > >>> +++ b/include/dt-bindings/media/media-bus-format.h
> > > >>> @@ -0,0 +1,177 @@
> > > >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> > > >>> +/*
> > > >>> + * Media Bus API header
> > > >>> + *
> > > >>> + * Copyright (C) 2009, Guennadi Liakhovetski
> > > >>> +<[email protected]>
> > > >>> + *
> > > >>> + * This program is free software; you can redistribute it and/or
> > > >>> +modify
> > > >>> + * it under the terms of the GNU General Public License version 2
> > > >>> +as
> > > >>> + * published by the Free Software Foundation.
> > > >>
> > > >> That's not true. Your SPDX tells something entirely different.
> > > >>
> > > >
> > > > Thank you - I'll see how to fix it.
> > > >
> > > >> Anyway, you did not explain why you need to copy anything anywhere.
> > > >>
> > > >> Specifically, random hex values *are not bindings*.
> > > >>
> > > >
> > > > The same media bus format values are being used by the reference
> > > > driver in patch #9. And, as far as I know, we cannot use headers from
> > > > Linux API headers directly (at least I
> > >
> > > I don't understand what does it mean. You can use in your driver
> whatever
> > > headers you wish, I don't care about them.
> > >
> > >
> > > noticed the same pattern in ../dt-bindings/sdtv-standarts.h for instance).
> > > What would be the best approach to reusing the same defines on DT and
> > > driver sides from your point of view? Symlink maybe?
> > > >
> > >
> > > Wrap your messages to match mailing list discussion style. There are no
> > > defines used in DT. If there are, show me them in *THIS* or other
> > > *upstreamed* (being upstreamed) patchset.
> > >
> >
> > Sorry, I didn't explain properly what I'm trying to achieve. I need to
> > create a DT node property that represents video signal format, one of
> > MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would
> be
> > nice to reuse the same symbolic values in the device tree. What is the
> > best approach here? Should I create a separate header in
> > include/dt-bindings with the same or similar (to avoid multiple
> > definition errors) defines, or is it better to create a symlink to
> > media-bus-format.h like include/dt-bindings/linux-event-codes.h?
>
> Isn't there already a property for this, described in
> Documentation/devicetree/bindings/media/xilinx/video.txt
> ?
Those properties are very similar, indeed but not exactly the same. The
one you noticed maps Xilinx video data format on AXI4-Stream transport
that covers color space and chroma subsampling only. The rest of the
signal attributes are either conventional or left out. MEDIA_BUS_FMT_*
collection is more specific and embeds additional information about
video signals, like bits per component and component ordering.
On 30/03/2024 03:02, Klymenko, Anatoliy wrote:
>>>>
>>>
>>> Sorry, I didn't explain properly what I'm trying to achieve. I need to
>>> create a DT node property that represents video signal format, one of
>>> MEDIA_BUS_FMT_* from include/uapi/linux/media-bus-format.h. It would
>> be
>>> nice to reuse the same symbolic values in the device tree. What is the
>>> best approach here? Should I create a separate header in
There is no user of this new header, so I don't agree. Please send
either full work or link your other upstreamed patchset. Anything sent
as "DO NOT MERGE" does not count because it is not an user.
Without the DTS user I claim that you do not bind here anything...
>>> include/dt-bindings with the same or similar (to avoid multiple
>>> definition errors) defines, or is it better to create a symlink to
>>> media-bus-format.h like include/dt-bindings/linux-event-codes.h?
Copying or symlinking entire header into bindings does not help us to
understand what is exactly a binding here.
For example, maybe you encode runtime information into DT (don't do
this) and that's why you need these defines... Or maybe your block has
some capabilities. Dunno, patch was not tested, is defined as do not
merge and is not explaining any of these.
Therefore, please provide complete set of users ready to be merged, test
your patches, provide rationale why this is supposed to be a binding and
why do you think it represents hardware configuration, not OS policy or
runtime configuration.
Best regards,
Krzysztof
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Set layer mode of operation (live or dma-based) during layer creation.
>
> Each DPSUB layer mode of operation is defined by corresponding DT node port
> connection, so it is possible to assign it during layer object creation.
> Previously it was set in layer enable functions, although it is too late
> as setting layer format depends on layer mode, and should be done before
> given layer enabled.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 ++++++++++++++++----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +------------
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 19 insertions(+), 18 deletions(-)
Reviewed-by: Tomi Valkeinen <[email protected]>
Tomi
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 8a39b3accce5..e6d26ef60e89 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -64,6 +64,16 @@
>
> #define ZYNQMP_DISP_MAX_NUM_SUB_PLANES 3
>
> +/**
> + * enum zynqmp_dpsub_layer_mode - Layer mode
> + * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> + * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> + */
> +enum zynqmp_dpsub_layer_mode {
> + ZYNQMP_DPSUB_LAYER_NONLIVE,
> + ZYNQMP_DPSUB_LAYER_LIVE,
> +};
> +
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> @@ -902,15 +912,12 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> /**
> * zynqmp_disp_layer_enable - Enable a layer
> * @layer: The layer
> - * @mode: Operating mode of layer
> *
> * Enable the @layer in the audio/video buffer manager and the blender. DMA
> * channels are started separately by zynqmp_disp_layer_update().
> */
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode)
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
> {
> - layer->mode = mode;
> zynqmp_disp_avbuf_enable_video(layer->disp, layer);
> zynqmp_disp_blend_layer_enable(layer->disp, layer);
> }
> @@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> layer->id = i;
> layer->disp = disp;
> layer->info = &layer_info[i];
> + /* For now assume dpsub works in either live or non-live mode for both layers.
> + * Hybrid mode is not supported yet.
> + */
> + layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> + : ZYNQMP_DPSUB_LAYER_LIVE;
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 123cffac08be..9b8b202224d9 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
> ZYNQMP_DPSUB_LAYER_GFX,
> };
>
> -/**
> - * enum zynqmp_dpsub_layer_mode - Layer mode
> - * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> - * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> - */
> -enum zynqmp_dpsub_layer_mode {
> - ZYNQMP_DPSUB_LAYER_NONLIVE,
> - ZYNQMP_DPSUB_LAYER_LIVE,
> -};
> -
> void zynqmp_disp_enable(struct zynqmp_disp *disp);
> void zynqmp_disp_disable(struct zynqmp_disp *disp);
> int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> @@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
>
> u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> unsigned int *num_formats);
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode);
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 1846c4971fd8..04b6bcac3b07 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1295,7 +1295,7 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> /* TODO: Make the format configurable. */
> info = drm_format_info(DRM_FORMAT_YUV422);
> zynqmp_disp_layer_set_format(layer, info);
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_LIVE);
> + zynqmp_disp_layer_enable(layer);
>
> if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index db3bb4afbfc4..43bf416b33d5 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -122,7 +122,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
>
> /* Enable or re-enable the plane if the format has changed. */
> if (format_changed)
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_NONLIVE);
> + zynqmp_disp_layer_enable(layer);
> }
>
> static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG register
> layout.
I think this description needs a bit more. Mention that the defines are
not currently used, so we can change them like this without any other
change.
Tomi
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> index f92a006d5070..fa3935384834 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> @@ -165,10 +165,10 @@
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 0x2
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12 0x3
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK GENMASK(2, 0)
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB 0x0
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 0x1
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 0x2
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY 0x3
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB (0x0 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 (0x1 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 (0x2 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY (0x3 << 4)
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_MASK GENMASK(5, 4)
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_CB_FIRST BIT(8)
> #define ZYNQMP_DISP_AV_BUF_PALETTE_MEMORY 0x400
>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Add a helper function capturing the first connected live display layer
> discovery logic.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 04b6bcac3b07..4faafdd76798 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1276,28 +1276,40 @@ static void zynqmp_dp_encoder_mode_set_stream(struct zynqmp_dp *dp,
> * DISP Configuration
> */
>
> +/**
> + * zynqmp_dp_disp_connected_live_layer - Return the first connected live layer
> + * @dp: DisplayPort IP core structure
> + *
> + * Return: The first connected live display layer or NULL if none of the live
> + * layer is connected.
"layers"
Reviewed-by: Tomi Valkeinen <[email protected]>
Tomi
> + */
> +static struct zynqmp_disp_layer *
> +zynqmp_dp_disp_connected_live_layer(struct zynqmp_dp *dp)
> +{
> + if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> + return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
> + else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> + return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
> + else
> + return NULL;
> +}
> +
> static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> struct drm_bridge_state *old_bridge_state)
> {
> - enum zynqmp_dpsub_layer_id layer_id;
> struct zynqmp_disp_layer *layer;
> const struct drm_format_info *info;
>
> - if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> - layer_id = ZYNQMP_DPSUB_LAYER_VID;
> - else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> - layer_id = ZYNQMP_DPSUB_LAYER_GFX;
> - else
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (!layer)
> return;
>
> - layer = dp->dpsub->layers[layer_id];
> -
> /* TODO: Make the format configurable. */
> info = drm_format_info(DRM_FORMAT_YUV422);
> zynqmp_disp_layer_set_format(layer, info);
> zynqmp_disp_layer_enable(layer);
>
> - if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> + if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
> else
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, false, 0);
> @@ -1310,11 +1322,8 @@ static void zynqmp_dp_disp_disable(struct zynqmp_dp *dp,
> {
> struct zynqmp_disp_layer *layer;
>
> - if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> - layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
> - else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> - layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
> - else
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (!layer)
> return;
>
> zynqmp_disp_disable(dp->dpsub->disp);
>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Set layer mode of operation (live or dma-based) during layer creation.
>
> Each DPSUB layer mode of operation is defined by corresponding DT node port
> connection, so it is possible to assign it during layer object creation.
> Previously it was set in layer enable functions, although it is too late
> as setting layer format depends on layer mode, and should be done before
> given layer enabled.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 ++++++++++++++++----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +------------
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 8a39b3accce5..e6d26ef60e89 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -64,6 +64,16 @@
>
> #define ZYNQMP_DISP_MAX_NUM_SUB_PLANES 3
>
> +/**
> + * enum zynqmp_dpsub_layer_mode - Layer mode
> + * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> + * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> + */
> +enum zynqmp_dpsub_layer_mode {
> + ZYNQMP_DPSUB_LAYER_NONLIVE,
> + ZYNQMP_DPSUB_LAYER_LIVE,
> +};
> +
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> @@ -902,15 +912,12 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> /**
> * zynqmp_disp_layer_enable - Enable a layer
> * @layer: The layer
> - * @mode: Operating mode of layer
> *
> * Enable the @layer in the audio/video buffer manager and the blender. DMA
> * channels are started separately by zynqmp_disp_layer_update().
> */
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode)
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
> {
> - layer->mode = mode;
> zynqmp_disp_avbuf_enable_video(layer->disp, layer);
> zynqmp_disp_blend_layer_enable(layer->disp, layer);
> }
> @@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> layer->id = i;
> layer->disp = disp;
> layer->info = &layer_info[i];
> + /* For now assume dpsub works in either live or non-live mode for both layers.
> + * Hybrid mode is not supported yet.
> + */
This comment style is not according to the style guide, and in fact you
fix it in the patch 4. So please fix it here instead.
Tomi
> + layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> + : ZYNQMP_DPSUB_LAYER_LIVE;
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 123cffac08be..9b8b202224d9 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
> ZYNQMP_DPSUB_LAYER_GFX,
> };
>
> -/**
> - * enum zynqmp_dpsub_layer_mode - Layer mode
> - * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> - * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> - */
> -enum zynqmp_dpsub_layer_mode {
> - ZYNQMP_DPSUB_LAYER_NONLIVE,
> - ZYNQMP_DPSUB_LAYER_LIVE,
> -};
> -
> void zynqmp_disp_enable(struct zynqmp_disp *disp);
> void zynqmp_disp_disable(struct zynqmp_disp *disp);
> int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> @@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
>
> u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> unsigned int *num_formats);
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode);
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 1846c4971fd8..04b6bcac3b07 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1295,7 +1295,7 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> /* TODO: Make the format configurable. */
> info = drm_format_info(DRM_FORMAT_YUV422);
> zynqmp_disp_layer_set_format(layer, info);
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_LIVE);
> + zynqmp_disp_layer_enable(layer);
>
> if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index db3bb4afbfc4..43bf416b33d5 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -122,7 +122,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
>
> /* Enable or re-enable the plane if the format has changed. */
> if (format_changed)
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_NONLIVE);
> + zynqmp_disp_layer_enable(layer);
> }
>
> static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> DPSUB in bridge mode supports multiple input media bus formats.
>
> Announce the list of supported input media bus formats via
> drm_bridge.atomic_get_input_bus_fmts callback.
> Introduce a set of live input formats, supported by DPSUB.
> Rename zynqmp_disp_layer_drm_formats() to zynqmp_disp_layer_formats() to
> reflect semantics for both live and non-live layer format lists.
>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 76 +++++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 31 ++++++++++++++++
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 101 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index e6d26ef60e89..abdc3971b193 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -18,6 +18,7 @@
> #include <linux/dma/xilinx_dpdma.h>
> #include <linux/dma-mapping.h>
> #include <linux/dmaengine.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -77,12 +78,14 @@ enum zynqmp_dpsub_layer_mode {
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> + * @bus_fmt: Media bus format
> * @buf_fmt: AV buffer format
> * @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats
> * @sf: Scaling factors for color components
> */
> struct zynqmp_disp_format {
> u32 drm_fmt;
> + u32 bus_fmt;
> u32 buf_fmt;
> bool swap;
> const u32 *sf;
> @@ -182,6 +185,12 @@ static const u32 scaling_factors_565[] = {
> ZYNQMP_DISP_AV_BUF_5BIT_SF,
> };
>
> +static const u32 scaling_factors_666[] = {
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> +};
> +
> static const u32 scaling_factors_888[] = {
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> @@ -364,6 +373,41 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
> },
> };
>
> +/* List of live video layer formats */
> +static const struct zynqmp_disp_format avbuf_live_fmts[] = {
> + {
> + .drm_fmt = DRM_FORMAT_RGB565,
> + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_6 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_666,
> + }, {
> + .drm_fmt = DRM_FORMAT_RGB888,
> + .bus_fmt = MEDIA_BUS_FMT_RGB888_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV422,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV444,
> + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_P210,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_101010,
> + },
> +};
> +
> static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg)
> {
> return readl(disp->avbuf.base + reg);
> @@ -883,16 +927,17 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> }
>
> /**
> - * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by the layer
> + * zynqmp_disp_layer_formats - Return DRM or media bus formats supported by
> + * the layer
> * @layer: The layer
> * @num_formats: Pointer to the returned number of formats
> *
> - * Return: A newly allocated u32 array that stores all the DRM formats
> + * Return: A newly allocated u32 array that stores all DRM or media bus formats
> * supported by the layer. The number of formats in the array is returned
> * through the num_formats argument.
> */
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats)
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats)
> {
> unsigned int i;
> u32 *formats;
> @@ -903,7 +948,9 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> return NULL;
>
> for (i = 0; i < layer->info->num_formats; ++i)
> - formats[i] = layer->info->formats[i].drm_fmt;
> + formats[i] = layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE
> + ? layer->info->formats[i].drm_fmt
> + : layer->info->formats[i].bus_fmt;
I find this quite confusing. Depending on the layer mode, you return
different format types. I think it's quite easy to use this kind of
function the wrong way.
Why not just make two separate functions?
Tomi
> *num_formats = layer->info->num_formats;
> return formats;
> @@ -1131,6 +1178,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> .num_channels = 1,
> },
> };
> + static const struct zynqmp_disp_layer_info live_layer_info = {
> + .formats = avbuf_live_fmts,
> + .num_formats = ARRAY_SIZE(avbuf_live_fmts),
> + .num_channels = 0,
> + };
>
> unsigned int i;
> int ret;
> @@ -1140,12 +1192,18 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
>
> layer->id = i;
> layer->disp = disp;
> - layer->info = &layer_info[i];
> - /* For now assume dpsub works in either live or non-live mode for both layers.
> +
> + /*
> + * For now assume dpsub works in either live or non-live mode for both layers.
> * Hybrid mode is not supported yet.
> */
> - layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> - : ZYNQMP_DPSUB_LAYER_LIVE;
> + if (disp->dpsub->dma_enabled) {
> + layer->mode = ZYNQMP_DPSUB_LAYER_NONLIVE;
> + layer->info = &layer_info[i];
> + } else {
> + layer->mode = ZYNQMP_DPSUB_LAYER_LIVE;
> + layer->info = &live_layer_info;
> + }
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 9b8b202224d9..88c285a12e23 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -50,8 +50,8 @@ int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
> bool enable, u32 alpha);
>
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats);
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats);
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 4faafdd76798..e3b9eb3d9273 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -22,6 +22,7 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -1577,6 +1578,35 @@ static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *brid
> return drm_edid_read_ddc(connector, &dp->aux.ddc);
> }
>
> +static u32 *zynqmp_dp_bridge_default_bus_fmts(unsigned int *num_input_fmts)
> +{
> + u32 *formats = kzalloc(sizeof(*formats), GFP_KERNEL);
> +
> + if (formats)
> + *formats = MEDIA_BUS_FMT_FIXED;
> + *num_input_fmts = !!formats;
> +
> + return formats;
> +}
> +
> +static u32 *
> +zynqmp_dp_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + struct zynqmp_dp *dp = bridge_to_dp(bridge);
> + struct zynqmp_disp_layer *layer;
> +
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (layer)
> + return zynqmp_disp_layer_formats(layer, num_input_fmts);
> + else
> + return zynqmp_dp_bridge_default_bus_fmts(num_input_fmts);
> +}
> +
> static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .attach = zynqmp_dp_bridge_attach,
> .detach = zynqmp_dp_bridge_detach,
> @@ -1589,6 +1619,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .atomic_check = zynqmp_dp_bridge_atomic_check,
> .detect = zynqmp_dp_bridge_detect,
> .edid_read = zynqmp_dp_bridge_edid_read,
> + .atomic_get_input_bus_fmts = zynqmp_dp_bridge_get_input_bus_fmts,
> };
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index 43bf416b33d5..bf9fba01df0e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -152,7 +152,7 @@ static int zynqmp_dpsub_create_planes(struct zynqmp_dpsub *dpsub)
> unsigned int num_formats;
> u32 *formats;
>
> - formats = zynqmp_disp_layer_drm_formats(layer, &num_formats);
> + formats = zynqmp_disp_layer_formats(layer, &num_formats);
> if (!formats)
> return -ENOMEM;
>
>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Avoid usage of global zynqmp_dpsub.dma_enabled flag in DPSUB layer
> context. This flag signals whether the driver runs in DRM CRTC or DRM
> bridge mode, assuming that all display layers share the same live or
> non-live mode of operation. Using per-layer mode instead of global flag
> will simplify future support of the hybrid scenario.
>
> Remove redundant checks in DMA request/release contexts as
> zynqmp_disp_layer.info is well-defined for all layer types, including the
> correct number of DMA channels required for each particular layer.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index abdc3971b193..0c2b3f4bffa6 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -980,7 +980,7 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> {
> unsigned int i;
>
> - if (layer->disp->dpsub->dma_enabled) {
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> for (i = 0; i < layer->drm_fmt->num_planes; i++)
> dmaengine_terminate_sync(layer->dmas[i].chan);
> }
> @@ -1006,7 +1006,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> /*
> @@ -1044,7 +1044,7 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info = layer->drm_fmt;
> unsigned int i;
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return 0;
>
> for (i = 0; i < info->num_planes; i++) {
> @@ -1094,9 +1094,6 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
> {
> unsigned int i;
>
> - if (!layer->info || !disp->dpsub->dma_enabled)
> - return;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
>
> @@ -1137,9 +1134,6 @@ static int zynqmp_disp_layer_request_dma(struct zynqmp_disp *disp,
> unsigned int i;
> int ret;
>
> - if (!disp->dpsub->dma_enabled)
> - return 0;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> char dma_channel_name[16];
>
Reviewed-by: Tomi Valkeinen <[email protected]>
Tomi
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Program live video input format according to selected media bus format.
>
> In the bridge mode of operation, DPSUB is connected to FPGA CRTC which
> almost certainly supports a single media bus format as its output. Expect
> this to be delivered via the new bridge atomic state. Program DPSUB
> registers accordingly. Update zynqmp_disp_layer_set_format() API to fit
> both live and non-live layer types.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 +++++---
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 0c2b3f4bffa6..a385d22d428e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
> const struct zynqmp_disp_format *fmt)
> {
> unsigned int i;
> - u32 val;
> + u32 val, reg;
>
> - val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> - val &= zynqmp_disp_layer_is_video(layer)
> - ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> - : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> - val |= fmt->buf_fmt;
> - zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
> + layer->disp_fmt = fmt;
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> + reg = ZYNQMP_DISP_AV_BUF_FMT;
> + val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> + val &= zynqmp_disp_layer_is_video(layer)
> + ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> + : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> + val |= fmt->buf_fmt;
> + } else {
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
> + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
> + val = fmt->buf_fmt;
> + }
> + zynqmp_disp_avbuf_write(disp, reg, val);
Just write the registers inside the above if-else blocks.
>
> for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
> - unsigned int reg = zynqmp_disp_layer_is_video(layer)
> - ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> - : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> + : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
>
> zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
> }
> @@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
> */
>
> /**
> - * zynqmp_disp_layer_find_format - Find format information for a DRM format
> + * zynqmp_disp_layer_find_format - Find format information for a DRM or media
> + * bus format
> * @layer: The layer
> - * @drm_fmt: DRM format to search
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Search display subsystem format information corresponding to the given DRM
> - * format @drm_fmt for the @layer, and return a pointer to the format
> - * descriptor.
> + * or media bus format @drm_or_bus_format for the @layer, and return a pointer
> + * to the format descriptor. Search key choice depends on @layer mode, for live
> + * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
> + * zynqmp_disp_format.drm_fmt is used.
Here also I recommend creating separate funcs for the fourcc and mbus
versions. They are different types, even if they happen to fit into u32.
> *
> * Return: A pointer to the format descriptor if found, NULL otherwise
> */
> static const struct zynqmp_disp_format *
> zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> - u32 drm_fmt)
> + u32 drm_or_bus_format)
> {
> unsigned int i;
> + const struct zynqmp_disp_format *disp_format;
>
> for (i = 0; i < layer->info->num_formats; i++) {
> - if (layer->info->formats[i].drm_fmt == drm_fmt)
> - return &layer->info->formats[i];
> + disp_format = &layer->info->formats[i];
> + if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
> + disp_format->bus_fmt == drm_or_bus_format) ||
> + (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
> + disp_format->drm_fmt == drm_or_bus_format))
> + return disp_format;
> }
>
> return NULL;
> @@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> /**
> * zynqmp_disp_layer_set_format - Set the layer format
> * @layer: The layer
> - * @info: The format info
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Set the format for @layer to @info. The layer must be disabled.
> */
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info)
> + u32 drm_or_bus_format)
And here, with a quick look, a separate function would be fine.
Tomi
> {
> unsigned int i;
>
> - layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
> - layer->drm_fmt = info;
> + layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
> + if (WARN_ON(!layer->disp_fmt))
> + return;
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> + layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
> + if (!layer->drm_fmt)
> + return;
> +
> if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> @@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> * Set pconfig for each DMA channel to indicate they're part of a
> * video group.
> */
> - for (i = 0; i < info->num_planes; i++) {
> + for (i = 0; i < layer->drm_fmt->num_planes; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> struct xilinx_dpdma_peripheral_config pconfig = {
> .video_group = true,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 88c285a12e23..9f9a5f50ffbc 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -55,7 +55,7 @@ u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info);
> + u32 drm_or_bus_format);
> int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> struct drm_plane_state *state);
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index e3b9eb3d9273..200e63636006 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> struct drm_bridge_state *old_bridge_state)
> {
> struct zynqmp_disp_layer *layer;
> - const struct drm_format_info *info;
> + struct drm_bridge_state *bridge_state;
> + u32 bus_fmt;
>
> layer = zynqmp_dp_disp_connected_live_layer(dp);
> if (!layer)
> return;
>
> - /* TODO: Make the format configurable. */
> - info = drm_format_info(DRM_FORMAT_YUV422);
> - zynqmp_disp_layer_set_format(layer, info);
> + bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
> + old_bridge_state->bridge);
> + if (WARN_ON(!bridge_state))
> + return;
> +
> + bus_fmt = bridge_state->input_bus_cfg.format;
> + zynqmp_disp_layer_set_format(layer, bus_fmt);
> zynqmp_disp_layer_enable(layer);
>
> if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index bf9fba01df0e..d96b3f3f2e3a 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -111,7 +111,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
> if (old_state->fb)
> zynqmp_disp_layer_disable(layer);
>
> - zynqmp_disp_layer_set_format(layer, new_state->fb->format);
> + zynqmp_disp_layer_set_format(layer, new_state->fb->format->format);
> }
>
> zynqmp_disp_layer_update(layer, new_state);
>
> -----Original Message-----
> From: Tomi Valkeinen <[email protected]>
> Sent: Friday, April 5, 2024 5:10 AM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Laurent Pinchart
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard
> <[email protected]>; Thomas Zimmermann <[email protected]>;
> David Airlie <[email protected]>; Daniel Vetter <[email protected]>;
> Simek, Michal <[email protected]>; Andrzej Hajda
> <[email protected]>; Neil Armstrong
> <[email protected]>; Robert Foss <[email protected]>; Jonas
> Karlman <[email protected]>; Jernej Skrabec
> <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Mauro Carvalho Chehab
> <[email protected]>
> Subject: Re: [PATCH v3 2/9] drm: xlnx: zynqmp_dpsub: Update live format
> defines
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> > Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG
> register
> > layout.
>
> I think this description needs a bit more. Mention that the defines are
> not currently used, so we can change them like this without any other
> change.
>
Makes sense. I'll update this.
> Tomi
>
> > Reviewed-by: Laurent Pinchart <[email protected]>
> > Signed-off-by: Anatoliy Klymenko <[email protected]>
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > index f92a006d5070..fa3935384834 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> > @@ -165,10 +165,10 @@
> > #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 0x2
> > #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12 0x3
> > #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK
> GENMASK(2, 0)
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB 0x0
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 0x1
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 0x2
> > -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY 0x3
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB (0x0
> << 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 (0x1 <<
> 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 (0x2 <<
> 4)
> > +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY (0x3 <<
> 4)
> > #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_MASK
> GENMASK(5, 4)
> > #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_CB_FIRST BIT(8)
> > #define ZYNQMP_DISP_AV_BUF_PALETTE_MEMORY 0x400
> >
> -----Original Message-----
> From: Tomi Valkeinen <[email protected]>
> Sent: Friday, April 5, 2024 5:31 AM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Laurent Pinchart
> <[email protected]>; Maarten Lankhorst
> <[email protected]>; Maxime Ripard
> <[email protected]>; Thomas Zimmermann <[email protected]>;
> David Airlie <[email protected]>; Daniel Vetter <[email protected]>;
> Simek, Michal <[email protected]>; Andrzej Hajda
> <[email protected]>; Neil Armstrong
> <[email protected]>; Robert Foss <[email protected]>; Jonas
> Karlman <[email protected]>; Jernej Skrabec
> <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Mauro Carvalho Chehab
> <[email protected]>
> Subject: Re: [PATCH v3 1/9] drm: xlnx: zynqmp_dpsub: Set layer mode
> during creation
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> > Set layer mode of operation (live or dma-based) during layer creation.
> >
> > Each DPSUB layer mode of operation is defined by corresponding DT
> node port
> > connection, so it is possible to assign it during layer object creation.
> > Previously it was set in layer enable functions, although it is too late
> > as setting layer format depends on layer mode, and should be done
> before
> > given layer enabled.
> >
> > Signed-off-by: Anatoliy Klymenko <[email protected]>
> > Reviewed-by: Laurent Pinchart <[email protected]>
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 ++++++++++++++++----
> > drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +------------
> > drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> > drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> > 4 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index 8a39b3accce5..e6d26ef60e89 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -64,6 +64,16 @@
> >
> > #define ZYNQMP_DISP_MAX_NUM_SUB_PLANES 3
> >
> > +/**
> > + * enum zynqmp_dpsub_layer_mode - Layer mode
> > + * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> > + * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> > + */
> > +enum zynqmp_dpsub_layer_mode {
> > + ZYNQMP_DPSUB_LAYER_NONLIVE,
> > + ZYNQMP_DPSUB_LAYER_LIVE,
> > +};
> > +
> > /**
> > * struct zynqmp_disp_format - Display subsystem format information
> > * @drm_fmt: DRM format (4CC)
> > @@ -902,15 +912,12 @@ u32
> *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> > /**
> > * zynqmp_disp_layer_enable - Enable a layer
> > * @layer: The layer
> > - * @mode: Operating mode of layer
> > *
> > * Enable the @layer in the audio/video buffer manager and the
> blender. DMA
> > * channels are started separately by zynqmp_disp_layer_update().
> > */
> > -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> > - enum zynqmp_dpsub_layer_mode mode)
> > +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
> > {
> > - layer->mode = mode;
> > zynqmp_disp_avbuf_enable_video(layer->disp, layer);
> > zynqmp_disp_blend_layer_enable(layer->disp, layer);
> > }
> > @@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct
> zynqmp_disp *disp)
> > layer->id = i;
> > layer->disp = disp;
> > layer->info = &layer_info[i];
> > + /* For now assume dpsub works in either live or non-live
> mode for both layers.
> > + * Hybrid mode is not supported yet.
> > + */
>
> This comment style is not according to the style guide, and in fact you
> fix it in the patch 4. So please fix it here instead.
>
Thanks for catching it.
> Tomi
>
> > + layer->mode = disp->dpsub->dma_enabled ?
> ZYNQMP_DPSUB_LAYER_NONLIVE
> > + : ZYNQMP_DPSUB_LAYER_LIVE;
> >
> > ret = zynqmp_disp_layer_request_dma(disp, layer);
> > if (ret)
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > index 123cffac08be..9b8b202224d9 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> > @@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
> > ZYNQMP_DPSUB_LAYER_GFX,
> > };
> >
> > -/**
> > - * enum zynqmp_dpsub_layer_mode - Layer mode
> > - * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> > - * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> > - */
> > -enum zynqmp_dpsub_layer_mode {
> > - ZYNQMP_DPSUB_LAYER_NONLIVE,
> > - ZYNQMP_DPSUB_LAYER_LIVE,
> > -};
> > -
> > void zynqmp_disp_enable(struct zynqmp_disp *disp);
> > void zynqmp_disp_disable(struct zynqmp_disp *disp);
> > int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> > @@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct
> zynqmp_disp *disp,
> >
> > u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer
> *layer,
> > unsigned int *num_formats);
> > -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> > - enum zynqmp_dpsub_layer_mode mode);
> > +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> > void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> > void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> > const struct drm_format_info *info);
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > index 1846c4971fd8..04b6bcac3b07 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > @@ -1295,7 +1295,7 @@ static void zynqmp_dp_disp_enable(struct
> zynqmp_dp *dp,
> > /* TODO: Make the format configurable. */
> > info = drm_format_info(DRM_FORMAT_YUV422);
> > zynqmp_disp_layer_set_format(layer, info);
> > - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_LIVE);
> > + zynqmp_disp_layer_enable(layer);
> >
> > if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> > zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true,
> 255);
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > index db3bb4afbfc4..43bf416b33d5 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> > @@ -122,7 +122,7 @@ static void
> zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
> >
> > /* Enable or re-enable the plane if the format has changed. */
> > if (format_changed)
> > - zynqmp_disp_layer_enable(layer,
> ZYNQMP_DPSUB_LAYER_NONLIVE);
> > + zynqmp_disp_layer_enable(layer);
> > }
> >
> > static const struct drm_plane_helper_funcs
> zynqmp_dpsub_plane_helper_funcs = {
> >