2024-03-13 00:55:20

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 0/8] Setting live video input format for ZynqMP DPSUB

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/8: Set the DPSUB layer mode of operation prior to enabling the
layer. Allows to use layer operational mode before its enablement.

Patch 2/8: Update some IP register defines.

Patch 3/8: Announce supported input media bus formats via
drm_bridge_funcs.atomic_get_input_bus_fmts callback.

Patch 4/8: Minimize usage of a global flag. Minor improvement.

Patch 5/8: Program DPSUB live video input format based on selected bus
config in the new atomic bridge state.

Patch 6/8: 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 7/8: DT bindings documentation for Video Timing Controller and Test
Pattern Generator IPs.

Patch 8/8: 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: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anatoliy Klymenko <[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 (8):
drm: xlnx: zynqmp_dpsub: Set layer mode during creation
drm: xlnx: zynqmp_dpsub: Update live format defines
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 | 854 +++++++++++++++++++++
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 | 187 ++++-
drivers/gpu/drm/xlnx/zynqmp_disp.h | 19 +-
drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 +-
drivers/gpu/drm/xlnx/zynqmp_dp.c | 41 +-
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, 2204 insertions(+), 63 deletions(-)
---
base-commit: bfa4437fd3938ae2e186e7664b2db65bb8775670
change-id: 20240226-dp-live-fmt-6415773b5a68

Best regards,
--
Anatoliy Klymenko <[email protected]>



2024-03-13 00:55:33

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 1/8] drm: xlnx: zynqmp_dpsub: Set layer mode during creation

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


2024-03-13 00:55:51

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 2/8] drm: xlnx: zynqmp_dpsub: Update live format defines

Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG register
layout.

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


2024-03-13 00:56:11

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 4/8] drm: xlnx: zynqmp_dpsub: Minimize usage of global flag

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 siplify future support of the hybrid scenario.

Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index af851190f447..dd48fa60fa9a 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -975,7 +975,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);
}
@@ -1001,7 +1001,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;

/*
@@ -1039,7 +1039,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++) {
@@ -1089,7 +1089,7 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
{
unsigned int i;

- if (!layer->info || !disp->dpsub->dma_enabled)
+ if (!layer->info)
return;

for (i = 0; i < layer->info->num_channels; i++) {
@@ -1132,9 +1132,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


2024-03-13 00:56:17

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 3/8] drm: xlnx: zynqmp_dpsub: Anounce supported input formats

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.

Signed-off-by: Anatoliy Klymenko <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 67 +++++++++++++++++++++++++++++++++-----
drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +--
drivers/gpu/drm/xlnx/zynqmp_dp.c | 26 +++++++++++++++
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
4 files changed, 88 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index e6d26ef60e89..af851190f447 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,16 @@ 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;
+ union {
+ u32 drm_fmt;
+ u32 bus_fmt;
+ };
u32 buf_fmt;
bool swap;
const u32 *sf;
@@ -182,6 +187,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 +375,36 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
},
};

+/* List of live video layer formats */
+static const struct zynqmp_disp_format avbuf_live_fmts[] = {
+ {
+ .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,
+ }, {
+ .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
+ .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
+ ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
+ .sf = scaling_factors_888,
+ }, {
+ .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,
+ }, {
+ .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,
+ }, {
+ .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 +924,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;
@@ -1131,6 +1173,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 +1187,16 @@ 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;
+ 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 04b6bcac3b07..a0d169ac48c0 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1568,6 +1568,31 @@ 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_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;
+ enum zynqmp_dpsub_layer_id layer_id;
+
+ 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 {
+ *num_input_fmts = 0;
+ return NULL;
+ }
+ layer = dp->dpsub->layers[layer_id];
+
+ return zynqmp_disp_layer_formats(layer, num_input_fmts);
+}
+
static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
.attach = zynqmp_dp_bridge_attach,
.detach = zynqmp_dp_bridge_detach,
@@ -1580,6 +1605,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


2024-03-13 00:56:18

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 5/8] drm: xlnx: zynqmp_dpsub: Set input live format

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 | 93 +++++++++++++++++++++++++++++++-------
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, 87 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index dd48fa60fa9a..0cacd597f4b8 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -383,7 +383,7 @@ static const struct zynqmp_disp_format avbuf_live_fmts[] = {
ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
.sf = scaling_factors_666,
}, {
- .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
+ .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,
.buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
.sf = scaling_factors_888,
@@ -433,19 +433,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]);
}
@@ -984,23 +993,73 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
zynqmp_disp_blend_layer_disable(layer->disp, layer);
}

+struct zynqmp_disp_bus_to_drm {
+ u32 bus_fmt;
+ u32 drm_fmt;
+};
+
+/**
+ * zynqmp_disp_reference_drm_format - Get reference DRM format corresponding
+ * to the given media bus format.
+ * @bus_format: Media bus format
+ *
+ * Map media bus format to some DRM format that represents the same color space
+ * and chroma subsampling as the @bus_format video signal. These DRM format
+ * properties are required to program the blender.
+ *
+ * Return: DRM format code corresponding to @bus_format
+ */
+static u32 zynqmp_disp_reference_drm_format(u32 bus_format)
+{
+ static const struct zynqmp_disp_bus_to_drm format_map[] = {
+ {
+ .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
+ .drm_fmt = DRM_FORMAT_RGB565,
+ }, {
+ .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,
+ .drm_fmt = DRM_FORMAT_RGB888,
+ }, {
+ .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
+ .drm_fmt = DRM_FORMAT_YUV422,
+ }, {
+ .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
+ .drm_fmt = DRM_FORMAT_YUV444,
+ }, {
+ .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
+ .drm_fmt = DRM_FORMAT_P210,
+ },
+ };
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(format_map); ++i)
+ if (format_map[i].bus_fmt == bus_format)
+ return format_map[i].drm_fmt;
+
+ return DRM_FORMAT_INVALID;
+}
+
/**
* 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);
zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);

+ if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
+ drm_or_bus_format = zynqmp_disp_reference_drm_format(drm_or_bus_format);
+
+ layer->drm_fmt = drm_format_info(drm_or_bus_format);
+ if (!layer->drm_fmt)
+ return;
+
if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
return;

@@ -1008,7 +1067,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 a0d169ac48c0..fc6b1d783c28 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1281,7 +1281,8 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
{
enum zynqmp_dpsub_layer_id layer_id;
struct zynqmp_disp_layer *layer;
- const struct drm_format_info *info;
+ struct drm_bridge_state *bridge_state;
+ u32 bus_fmt;

if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
layer_id = ZYNQMP_DPSUB_LAYER_VID;
@@ -1291,10 +1292,14 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
return;

layer = dp->dpsub->layers[layer_id];
+ 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);

- /* 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)
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


2024-03-13 00:56:41

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 6/8] drm/atomic-helper: Add select_output_bus_format callback

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


2024-03-13 00:57:07

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 7/8] dt-bindings: xlnx: Add VTC and TPG bindings

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


2024-03-13 00:57:32

by Anatoliy Klymenko

[permalink] [raw]
Subject: [PATCH v2 8/8] drm: xlnx: Intoduce TPG CRTC driver

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 | 854 +++++++++++++++++++++++++++++++++++
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, 1592 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..297a65fdb289
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_tpg.c
@@ -0,0 +1,854 @@
+// 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 encoder
+ */
+
+static const struct drm_encoder_funcs xlnx_tpg_encoder_funcs_cleanup = {
+ .destroy = drm_encoder_cleanup,
+};
+
+/* -----------------------------------------------------------------------------
+ * 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


2024-03-13 02:31:02

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] dt-bindings: xlnx: Add VTC and TPG bindings


On Tue, 12 Mar 2024 17:55:04 -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,vtc.yaml: xlnx,pixels-per-clock: missing type definition
/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

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.


2024-03-13 07:28:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] dt-bindings: xlnx: Add VTC and TPG bindings

On 13/03/2024 01:55, Anatoliy Klymenko wrote:
> DO NOT MERGE. REFERENCE ONLY.

Please add it to the PATCH title, so [PATCH DO NOT MERGE]... but better
just don't send it in one thread. Many maintainers apply entire thread,
thus whatever you send here, would be applied.

I am not going to review this patch, because as you said, should not be
merged, so just for formality:

NAK.

Best regards,
Krzysztof


2024-03-14 12:05:22

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] drm: xlnx: Intoduce TPG CRTC driver

Hi,

On Tue, Mar 12, 2024 at 05:55:05PM -0700, Anatoliy Klymenko wrote:
> 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]>

As I said previously, we don't want to have unused APIs, so this patch
should be in a good enough state to be merged if we want to merge the
whole API.

> +/* -----------------------------------------------------------------------------
> + * 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 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,
> +};

From that code, it's not clear to me how the CRTC is going to be able to
get what the format is.

It looks like you hardcode it here, but what if there's several that
would fit the bill? Is the CRTC expected to store it into its private
structure?

If so, I would expect it to be in the crtc state, and atomic_enable to
just reuse whatever is in the state.

Maxime


Attachments:
(No filename) (2.56 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-14 19:43:57

by Anatoliy Klymenko

[permalink] [raw]
Subject: RE: [PATCH v2 8/8] drm: xlnx: Intoduce TPG CRTC driver

Hi Maxime,

Thank you for the review.

> -----Original Message-----
> From: Maxime Ripard <[email protected]>
> Sent: Thursday, March 14, 2024 5:05 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]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 8/8] drm: xlnx: Intoduce TPG CRTC driver
>
> Hi,
>
> On Tue, Mar 12, 2024 at 05:55:05PM -0700, Anatoliy Klymenko wrote:
> > 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]>
>
> As I said previously, we don't want to have unused APIs, so this patch should be in
> a good enough state to be merged if we want to merge the whole API.
>

This is understandable, but even having this API just reviewed by the community will open the path forward for aligning AMD/Xilinx downstream DRM drivers with the upstream kernel.

> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * 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 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,
> > +};
>
> From that code, it's not clear to me how the CRTC is going to be able to get what
> the format is.
>

It's coming from DT "bus-format" property. The idea is that this property will reflect the FPGA design variation synthesized.

> It looks like you hardcode it here, but what if there's several that would fit the
> bill? Is the CRTC expected to store it into its private structure?
>

It's impractical from the resources utilization point of view to support multiple runtime options for FPGA-based CRTCs output signal format, so the bus-format will be runtime fixed but can vary between differently synthesized instances.

> If so, I would expect it to be in the crtc state, and atomic_enable to just reuse
> whatever is in the state.
>

This could be totally valid for different kinds of CRTCs, although for this particular case, the bus-fomat choice is runtime immutable.

> Maxime

Thank you,
Anatoliy

2024-03-15 15:24:52

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] drm: xlnx: Intoduce TPG CRTC driver

On Thu, Mar 14, 2024 at 07:43:30PM +0000, Klymenko, Anatoliy wrote:
> > > +/*
> > > +---------------------------------------------------------------------
> > > +--------
> > > + * 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 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,
> > > +};
> >
> > From that code, it's not clear to me how the CRTC is going to be able to get what
> > the format is.
> >
>
> It's coming from DT "bus-format" property. The idea is that this
> property will reflect the FPGA design variation synthesized.
>
> > It looks like you hardcode it here, but what if there's several that
> > would fit the bill? Is the CRTC expected to store it into its
> > private structure?
> >
>
> It's impractical from the resources utilization point of view to
> support multiple runtime options for FPGA-based CRTCs output signal
> format, so the bus-format will be runtime fixed but can vary between
> differently synthesized instances.
>
> > If so, I would expect it to be in the crtc state, and atomic_enable to just reuse
> > whatever is in the state.
> >
>
> This could be totally valid for different kinds of CRTCs, although for
> this particular case, the bus-fomat choice is runtime immutable.

Sure, but we're still discussing an API to accomodate your use-case
here. Your usecase is one thing, but the API has to be cover all cases,
and there's definitely some CRTCs out there that support multiple output
formats that would benefit from that API.

And it would mimic the drm_bridge API, which is a nice consistency
bonus.

Maxime


Attachments:
(No filename) (3.05 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-18 23:17:00

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v2 2/8] drm: xlnx: zynqmp_dpsub: Update live format defines

Hi Anatoliy,

Thank you for the patch.

On Tue, Mar 12, 2024 at 05:54:59PM -0700, Anatoliy Klymenko wrote:
> Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG register
> layout.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>

Reviewed-by: Laurent Pinchart <[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
>

--
Regards,

Laurent Pinchart

2024-03-19 00:05:46

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v2 3/8] drm: xlnx: zynqmp_dpsub: Anounce supported input formats

Hi Anatoliy,

Thank you for the patch.

On Tue, Mar 12, 2024 at 05:55:00PM -0700, 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.
>
> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 67 +++++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +--
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 26 +++++++++++++++
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index e6d26ef60e89..af851190f447 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,16 @@ 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;
> + union {
> + u32 drm_fmt;
> + u32 bus_fmt;
> + };

I'm not a big fan of the union, but I can live with it.

> u32 buf_fmt;
> bool swap;
> const u32 *sf;
> @@ -182,6 +187,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 +375,36 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
> },
> };
>
> +/* List of live video layer formats */
> +static const struct zynqmp_disp_format avbuf_live_fmts[] = {
> + {
> + .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,
> + }, {
> + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_888,
> + }, {
> + .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,
> + }, {
> + .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,
> + }, {
> + .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 +924,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;
> @@ -1131,6 +1173,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 +1187,16 @@ 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.

While are it, could you please turn this into

/*
* For now assume dpsub works in either live or non-live mode for both layers.

with a blank line just above it ?

> * 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 04b6bcac3b07..a0d169ac48c0 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1568,6 +1568,31 @@ 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_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;
> + enum zynqmp_dpsub_layer_id layer_id;
> +
> + 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 {
> + *num_input_fmts = 0;
> + return NULL;
> + }

You need curly braces around all branches if one of them has multiple
statements.

Given that the above pattern is repeated twice already, a helper
function that returns the layer pointer would be useful. Then you could
simply write

layer = ...(dp);
if (!layer) {
*num_input_fmts = 0;
return NULL;
}

With these small issues addressed,

Reviewed-by: Laurent Pinchart <[email protected]>

> + layer = dp->dpsub->layers[layer_id];
> +
> + return zynqmp_disp_layer_formats(layer, num_input_fmts);
> +}
> +
> static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .attach = zynqmp_dp_bridge_attach,
> .detach = zynqmp_dp_bridge_detach,
> @@ -1580,6 +1605,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;
>
>

--
Regards,

Laurent Pinchart

2024-03-19 00:13:13

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] drm: xlnx: zynqmp_dpsub: Minimize usage of global flag

Hi Anatoliy,

Thank you for the patch.

On Tue, Mar 12, 2024 at 05:55:01PM -0700, 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 siplify future support of the hybrid scenario.

s/siplify/simplify/

> Signed-off-by: Anatoliy Klymenko <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index af851190f447..dd48fa60fa9a 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -975,7 +975,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);
> }
> @@ -1001,7 +1001,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;
>
> /*
> @@ -1039,7 +1039,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;

The above changes look nice.

>
> for (i = 0; i < info->num_planes; i++) {
> @@ -1089,7 +1089,7 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
> {
> unsigned int i;
>
> - if (!layer->info || !disp->dpsub->dma_enabled)
> + if (!layer->info)

This, however, doesn't seem right, as this function is called
unconditionally from the remove path. The change below seems weird too.
If I'm missing something, it should at least be explained in the commit
message.

> return;
>
> for (i = 0; i < layer->info->num_channels; i++) {
> @@ -1132,9 +1132,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];
>

--
Regards,

Laurent Pinchart

2024-03-19 00:35:45

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] drm: xlnx: zynqmp_dpsub: Set input live format

Hi Anatoliy,

Thank you for the patch.

On Tue, Mar 12, 2024 at 05:55:02PM -0700, 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.

No line breaks within paragraphs. Add a blank line if you want to
paragraphs, remove the line break otherwise.

> 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 | 93 +++++++++++++++++++++++++++++++-------
> 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, 87 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index dd48fa60fa9a..0cacd597f4b8 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -383,7 +383,7 @@ static const struct zynqmp_disp_format avbuf_live_fmts[] = {
> ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> .sf = scaling_factors_666,
> }, {
> - .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
> + .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,

Does this belong to a previous patch ?

> .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> .sf = scaling_factors_888,
> @@ -433,19 +433,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]);
> }
> @@ -984,23 +993,73 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> zynqmp_disp_blend_layer_disable(layer->disp, layer);
> }
>
> +struct zynqmp_disp_bus_to_drm {
> + u32 bus_fmt;
> + u32 drm_fmt;
> +};
> +
> +/**
> + * zynqmp_disp_reference_drm_format - Get reference DRM format corresponding
> + * to the given media bus format.
> + * @bus_format: Media bus format
> + *
> + * Map media bus format to some DRM format that represents the same color space
> + * and chroma subsampling as the @bus_format video signal. These DRM format
> + * properties are required to program the blender.
> + *
> + * Return: DRM format code corresponding to @bus_format
> + */
> +static u32 zynqmp_disp_reference_drm_format(u32 bus_format)
> +{
> + static const struct zynqmp_disp_bus_to_drm format_map[] = {
> + {
> + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
> + .drm_fmt = DRM_FORMAT_RGB565,
> + }, {
> + .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,
> + .drm_fmt = DRM_FORMAT_RGB888,
> + }, {
> + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
> + .drm_fmt = DRM_FORMAT_YUV422,
> + }, {
> + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
> + .drm_fmt = DRM_FORMAT_YUV444,
> + }, {
> + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
> + .drm_fmt = DRM_FORMAT_P210,
> + },

Hmmmm... Looking at this, I think you should have both bus_fmt and
drm_fmt in zynqmp_disp_format. It seems it would simplify the code flow
and make things more readable. If you would like me to give it a try,
please let me know.

> + };
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(format_map); ++i)
> + if (format_map[i].bus_fmt == bus_format)
> + return format_map[i].drm_fmt;
> +
> + return DRM_FORMAT_INVALID;
> +}
> +
> /**
> * 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);
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> + drm_or_bus_format = zynqmp_disp_reference_drm_format(drm_or_bus_format);
> +
> + layer->drm_fmt = drm_format_info(drm_or_bus_format);
> + if (!layer->drm_fmt)
> + return;
> +
> if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> @@ -1008,7 +1067,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 a0d169ac48c0..fc6b1d783c28 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1281,7 +1281,8 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> {
> enum zynqmp_dpsub_layer_id layer_id;
> struct zynqmp_disp_layer *layer;
> - const struct drm_format_info *info;
> + struct drm_bridge_state *bridge_state;
> + u32 bus_fmt;
>
> if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> layer_id = ZYNQMP_DPSUB_LAYER_VID;
> @@ -1291,10 +1292,14 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> return;
>
> layer = dp->dpsub->layers[layer_id];
> + 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);
>
> - /* 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)
> 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);
>

--
Regards,

Laurent Pinchart

2024-03-20 00:58:17

by Anatoliy Klymenko

[permalink] [raw]
Subject: RE: [PATCH v2 3/8] drm: xlnx: zynqmp_dpsub: Anounce supported input formats

Hi Laurent,

Thanks a lot for the review.

> -----Original Message-----
> From: Laurent Pinchart <[email protected]>
> Sent: Monday, March 18, 2024 5:05 PM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: 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]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/8] drm: xlnx: zynqmp_dpsub: Anounce supported input
> formats
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Anatoliy,
>
> Thank you for the patch.
>
> On Tue, Mar 12, 2024 at 05:55:00PM -0700, 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.
> >
> > Signed-off-by: Anatoliy Klymenko <[email protected]>
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_disp.c | 67
> > +++++++++++++++++++++++++++++++++-----
> > drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +--
> > drivers/gpu/drm/xlnx/zynqmp_dp.c | 26 +++++++++++++++
> > drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> > 4 files changed, 88 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index e6d26ef60e89..af851190f447 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,16 @@ 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;
> > + union {
> > + u32 drm_fmt;
> > + u32 bus_fmt;
> > + };
>
> I'm not a big fan of the union, but I can live with it.
>

I'm trying to represent the duality of the layer formats - non-live described by the DRM fourcc, and live by the bus format.

> > u32 buf_fmt;
> > bool swap;
> > const u32 *sf;
> > @@ -182,6 +187,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 +375,36 @@ static const struct zynqmp_disp_format
> avbuf_gfx_fmts[] = {
> > },
> > };
> >
> > +/* List of live video layer formats */ static const struct
> > +zynqmp_disp_format avbuf_live_fmts[] = {
> > + {
> > + .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,
> > + }, {
> > + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
> > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> > + .sf = scaling_factors_888,
> > + }, {
> > + .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,
> > + }, {
> > + .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,
> > + }, {
> > + .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 +924,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;
> > @@ -1131,6 +1173,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 +1187,16 @@ 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.
>
> While are it, could you please turn this into
>
> /*
> * For now assume dpsub works in either live or non-live mode for both
> layers.
>
> with a blank line just above it ?
>

Sure, thank you.

> > * 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 04b6bcac3b07..a0d169ac48c0 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > @@ -1568,6 +1568,31 @@ 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_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;
> > + enum zynqmp_dpsub_layer_id layer_id;
> > +
> > + 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 {
> > + *num_input_fmts = 0;
> > + return NULL;
> > + }
>
> You need curly braces around all branches if one of them has multiple statements.
>

Hmm, checkpatch gave me a CHECK hint on that. How did I miss it? Will fix. Thank you.

> Given that the above pattern is repeated twice already, a helper function that
> returns the layer pointer would be useful. Then you could simply write
>
> layer = ...(dp);
> if (!layer) {
> *num_input_fmts = 0;
> return NULL;
> }
>

Makes sense - I'll fix that.

> With these small issues addressed,
>
> Reviewed-by: Laurent Pinchart <[email protected]>
>
> > + layer = dp->dpsub->layers[layer_id];
> > +
> > + return zynqmp_disp_layer_formats(layer, num_input_fmts); }
> > +
> > static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> > .attach = zynqmp_dp_bridge_attach,
> > .detach = zynqmp_dp_bridge_detach, @@ -1580,6 +1605,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;
> >
> >
>
> --
> Regards,
>
> Laurent Pinchart

2024-03-20 01:12:48

by Anatoliy Klymenko

[permalink] [raw]
Subject: RE: [PATCH v2 4/8] drm: xlnx: zynqmp_dpsub: Minimize usage of global flag

Hi Laurent,

Thank you for the review.

> -----Original Message-----
> From: Laurent Pinchart <[email protected]>
> Sent: Monday, March 18, 2024 5:13 PM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: 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]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 4/8] drm: xlnx: zynqmp_dpsub: Minimize usage of global
> flag
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Anatoliy,
>
> Thank you for the patch.
>
> On Tue, Mar 12, 2024 at 05:55:01PM -0700, 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 siplify future support of the hybrid scenario.
>
> s/siplify/simplify/
>
> > Signed-off-by: Anatoliy Klymenko <[email protected]>
> > ---
> > drivers/gpu/drm/xlnx/zynqmp_disp.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index af851190f447..dd48fa60fa9a 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -975,7 +975,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);
> > }
> > @@ -1001,7 +1001,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;
> >
> > /*
> > @@ -1039,7 +1039,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;
>
> The above changes look nice.
>
> >
> > for (i = 0; i < info->num_planes; i++) { @@ -1089,7 +1089,7 @@
> > static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp, {
> > unsigned int i;
> >
> > - if (!layer->info || !disp->dpsub->dma_enabled)
> > + if (!layer->info)
>
> This, however, doesn't seem right, as this function is called unconditionally from
> the remove path. The change below seems weird too.
> If I'm missing something, it should at least be explained in the commit message.
>

Actually, this whole condition should be removed, as now we're setting layer.info for all types of layers. On top of that, we're setting the number of DMA channels to zero for the live layers, which in turn prevents any DMA channel initialization or release. You are right - that probably should be mentioned explicitly in the commit message. I'll update it.

> > return;
> >
> > for (i = 0; i < layer->info->num_channels; i++) { @@ -1132,9
> > +1132,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];
> >
>
> --
> Regards,
>
> Laurent Pinchart

Thank you,
Anatoliy

2024-03-20 01:25:57

by Anatoliy Klymenko

[permalink] [raw]
Subject: RE: [PATCH v2 5/8] drm: xlnx: zynqmp_dpsub: Set input live format

Hi Laurent,

Thanks a lot for your review.

> -----Original Message-----
> From: Laurent Pinchart <[email protected]>
> Sent: Monday, March 18, 2024 5:35 PM
> To: Klymenko, Anatoliy <[email protected]>
> Cc: 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]>; dri-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 5/8] drm: xlnx: zynqmp_dpsub: Set input live format
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Anatoliy,
>
> Thank you for the patch.
>
> On Tue, Mar 12, 2024 at 05:55:02PM -0700, 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.
>
> No line breaks within paragraphs. Add a blank line if you want to paragraphs,
> remove the line break otherwise.
>

Got it - will fix. Thank you.

> > 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 | 93
> > +++++++++++++++++++++++++++++++-------
> > 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, 87 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > index dd48fa60fa9a..0cacd597f4b8 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> > @@ -383,7 +383,7 @@ static const struct zynqmp_disp_format
> avbuf_live_fmts[] = {
> > ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> > .sf = scaling_factors_666,
> > }, {
> > - .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24,
> > + .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,
>
> Does this belong to a previous patch ?

Yep, slipped between my fingers during the rebase. I will update this in the next version.

>
> > .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> > ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> > .sf = scaling_factors_888,
> > @@ -433,19 +433,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]);
> > }
> > @@ -984,23 +993,73 @@ void zynqmp_disp_layer_disable(struct
> zynqmp_disp_layer *layer)
> > zynqmp_disp_blend_layer_disable(layer->disp, layer); }
> >
> > +struct zynqmp_disp_bus_to_drm {
> > + u32 bus_fmt;
> > + u32 drm_fmt;
> > +};
> > +
> > +/**
> > + * zynqmp_disp_reference_drm_format - Get reference DRM format
> > +corresponding
> > + * to the given media bus format.
> > + * @bus_format: Media bus format
> > + *
> > + * Map media bus format to some DRM format that represents the same
> > +color space
> > + * and chroma subsampling as the @bus_format video signal. These DRM
> > +format
> > + * properties are required to program the blender.
> > + *
> > + * Return: DRM format code corresponding to @bus_format */ static
> > +u32 zynqmp_disp_reference_drm_format(u32 bus_format) {
> > + static const struct zynqmp_disp_bus_to_drm format_map[] = {
> > + {
> > + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
> > + .drm_fmt = DRM_FORMAT_RGB565,
> > + }, {
> > + .bus_fmt = MEDIA_BUS_FMT_RBG888_1X24,
> > + .drm_fmt = DRM_FORMAT_RGB888,
> > + }, {
> > + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
> > + .drm_fmt = DRM_FORMAT_YUV422,
> > + }, {
> > + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
> > + .drm_fmt = DRM_FORMAT_YUV444,
> > + }, {
> > + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
> > + .drm_fmt = DRM_FORMAT_P210,
> > + },
>
> Hmmmm... Looking at this, I think you should have both bus_fmt and drm_fmt in
> zynqmp_disp_format. It seems it would simplify the code flow and make things
> more readable. If you would like me to give it a try, please let me know.
>

I was trying to preserve symmetry in struct zynqmp_disp_format, having only relevant information there for both layer modes, but this ended up in this ugly lookup. Probably the proper fix would be adding blender register values to this struct, but that would be a rather big change. So, I think your proposal is a good compromise. This will require a few layer.mode checks though. Thank you.

> > + };
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(format_map); ++i)
> > + if (format_map[i].bus_fmt == bus_format)
> > + return format_map[i].drm_fmt;
> > +
> > + return DRM_FORMAT_INVALID;
> > +}
> > +
> > /**
> > * 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);
> > zynqmp_disp_avbuf_set_format(layer->disp, layer,
> > layer->disp_fmt);
> >
> > + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> > + drm_or_bus_format =
> > + zynqmp_disp_reference_drm_format(drm_or_bus_format);
> > +
> > + layer->drm_fmt = drm_format_info(drm_or_bus_format);
> > + if (!layer->drm_fmt)
> > + return;
> > +
> > if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> > return;
> >
> > @@ -1008,7 +1067,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 a0d169ac48c0..fc6b1d783c28 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > @@ -1281,7 +1281,8 @@ static void zynqmp_dp_disp_enable(struct
> > zynqmp_dp *dp, {
> > enum zynqmp_dpsub_layer_id layer_id;
> > struct zynqmp_disp_layer *layer;
> > - const struct drm_format_info *info;
> > + struct drm_bridge_state *bridge_state;
> > + u32 bus_fmt;
> >
> > if (dp->dpsub->connected_ports &
> BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> > layer_id = ZYNQMP_DPSUB_LAYER_VID; @@ -1291,10 +1292,14
> > @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> > return;
> >
> > layer = dp->dpsub->layers[layer_id];
> > + 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);
> >
> > - /* 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) 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);
> >
>
> --
> Regards,
>
> Laurent Pinchart

Thank you,
Anatoliy