[v3,6/9] drm: xlnx: zynqmp_dpsub: Set input live format
Commit Message
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 <anatoliy.klymenko@amd.com>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 +-
drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 +++++---
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
4 files changed, 55 insertions(+), 28 deletions(-)
Comments
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Program live video input format according to selected media bus format.
>
> In the bridge mode of operation, DPSUB is connected to FPGA CRTC which
> almost certainly supports a single media bus format as its output. Expect
> this to be delivered via the new bridge atomic state. Program DPSUB
> registers accordingly. Update zynqmp_disp_layer_set_format() API to fit
> both live and non-live layer types.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 66 +++++++++++++++++++++++++-------------
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 +++++---
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 0c2b3f4bffa6..a385d22d428e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
> const struct zynqmp_disp_format *fmt)
> {
> unsigned int i;
> - u32 val;
> + u32 val, reg;
>
> - val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> - val &= zynqmp_disp_layer_is_video(layer)
> - ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> - : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> - val |= fmt->buf_fmt;
> - zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
> + layer->disp_fmt = fmt;
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> + reg = ZYNQMP_DISP_AV_BUF_FMT;
> + val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
> + val &= zynqmp_disp_layer_is_video(layer)
> + ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
> + : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
> + val |= fmt->buf_fmt;
> + } else {
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
> + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
> + val = fmt->buf_fmt;
> + }
> + zynqmp_disp_avbuf_write(disp, reg, val);
Just write the registers inside the above if-else blocks.
>
> for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
> - unsigned int reg = zynqmp_disp_layer_is_video(layer)
> - ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> - : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
> + reg = zynqmp_disp_layer_is_video(layer)
> + ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
> + : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
>
> zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
> }
> @@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
> */
>
> /**
> - * zynqmp_disp_layer_find_format - Find format information for a DRM format
> + * zynqmp_disp_layer_find_format - Find format information for a DRM or media
> + * bus format
> * @layer: The layer
> - * @drm_fmt: DRM format to search
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Search display subsystem format information corresponding to the given DRM
> - * format @drm_fmt for the @layer, and return a pointer to the format
> - * descriptor.
> + * or media bus format @drm_or_bus_format for the @layer, and return a pointer
> + * to the format descriptor. Search key choice depends on @layer mode, for live
> + * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
> + * zynqmp_disp_format.drm_fmt is used.
Here also I recommend creating separate funcs for the fourcc and mbus
versions. They are different types, even if they happen to fit into u32.
> *
> * Return: A pointer to the format descriptor if found, NULL otherwise
> */
> static const struct zynqmp_disp_format *
> zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> - u32 drm_fmt)
> + u32 drm_or_bus_format)
> {
> unsigned int i;
> + const struct zynqmp_disp_format *disp_format;
>
> for (i = 0; i < layer->info->num_formats; i++) {
> - if (layer->info->formats[i].drm_fmt == drm_fmt)
> - return &layer->info->formats[i];
> + disp_format = &layer->info->formats[i];
> + if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
> + disp_format->bus_fmt == drm_or_bus_format) ||
> + (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
> + disp_format->drm_fmt == drm_or_bus_format))
> + return disp_format;
> }
>
> return NULL;
> @@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> /**
> * zynqmp_disp_layer_set_format - Set the layer format
> * @layer: The layer
> - * @info: The format info
> + * @drm_or_bus_format: DRM or media bus format
> *
> * Set the format for @layer to @info. The layer must be disabled.
> */
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info)
> + u32 drm_or_bus_format)
And here, with a quick look, a separate function would be fine.
Tomi
> {
> unsigned int i;
>
> - layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
> - layer->drm_fmt = info;
> + layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
> + if (WARN_ON(!layer->disp_fmt))
> + return;
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> + layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
> + if (!layer->drm_fmt)
> + return;
> +
> if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> @@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> * Set pconfig for each DMA channel to indicate they're part of a
> * video group.
> */
> - for (i = 0; i < info->num_planes; i++) {
> + for (i = 0; i < layer->drm_fmt->num_planes; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> struct xilinx_dpdma_peripheral_config pconfig = {
> .video_group = true,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 88c285a12e23..9f9a5f50ffbc 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -55,7 +55,7 @@ u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> - const struct drm_format_info *info);
> + u32 drm_or_bus_format);
> int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> struct drm_plane_state *state);
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index e3b9eb3d9273..200e63636006 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> struct drm_bridge_state *old_bridge_state)
> {
> struct zynqmp_disp_layer *layer;
> - const struct drm_format_info *info;
> + struct drm_bridge_state *bridge_state;
> + u32 bus_fmt;
>
> layer = zynqmp_dp_disp_connected_live_layer(dp);
> if (!layer)
> return;
>
> - /* TODO: Make the format configurable. */
> - info = drm_format_info(DRM_FORMAT_YUV422);
> - zynqmp_disp_layer_set_format(layer, info);
> + bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
> + old_bridge_state->bridge);
> + if (WARN_ON(!bridge_state))
> + return;
> +
> + bus_fmt = bridge_state->input_bus_cfg.format;
> + zynqmp_disp_layer_set_format(layer, bus_fmt);
> zynqmp_disp_layer_enable(layer);
>
> if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index bf9fba01df0e..d96b3f3f2e3a 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -111,7 +111,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
> if (old_state->fb)
> zynqmp_disp_layer_disable(layer);
>
> - zynqmp_disp_layer_set_format(layer, new_state->fb->format);
> + zynqmp_disp_layer_set_format(layer, new_state->fb->format->format);
> }
>
> zynqmp_disp_layer_update(layer, new_state);
>
@@ -436,19 +436,28 @@ static void zynqmp_disp_avbuf_set_format(struct zynqmp_disp *disp,
const struct zynqmp_disp_format *fmt)
{
unsigned int i;
- u32 val;
+ u32 val, reg;
- val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
- val &= zynqmp_disp_layer_is_video(layer)
- ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
- : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
- val |= fmt->buf_fmt;
- zynqmp_disp_avbuf_write(disp, ZYNQMP_DISP_AV_BUF_FMT, val);
+ layer->disp_fmt = fmt;
+ if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
+ reg = ZYNQMP_DISP_AV_BUF_FMT;
+ val = zynqmp_disp_avbuf_read(disp, ZYNQMP_DISP_AV_BUF_FMT);
+ val &= zynqmp_disp_layer_is_video(layer)
+ ? ~ZYNQMP_DISP_AV_BUF_FMT_NL_VID_MASK
+ : ~ZYNQMP_DISP_AV_BUF_FMT_NL_GFX_MASK;
+ val |= fmt->buf_fmt;
+ } else {
+ reg = zynqmp_disp_layer_is_video(layer)
+ ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG
+ : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG;
+ val = fmt->buf_fmt;
+ }
+ zynqmp_disp_avbuf_write(disp, reg, val);
for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; i++) {
- unsigned int reg = zynqmp_disp_layer_is_video(layer)
- ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
- : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
+ reg = zynqmp_disp_layer_is_video(layer)
+ ? ZYNQMP_DISP_AV_BUF_VID_COMP_SF(i)
+ : ZYNQMP_DISP_AV_BUF_GFX_COMP_SF(i);
zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]);
}
@@ -902,25 +911,33 @@ static void zynqmp_disp_audio_disable(struct zynqmp_disp *disp)
*/
/**
- * zynqmp_disp_layer_find_format - Find format information for a DRM format
+ * zynqmp_disp_layer_find_format - Find format information for a DRM or media
+ * bus format
* @layer: The layer
- * @drm_fmt: DRM format to search
+ * @drm_or_bus_format: DRM or media bus format
*
* Search display subsystem format information corresponding to the given DRM
- * format @drm_fmt for the @layer, and return a pointer to the format
- * descriptor.
+ * or media bus format @drm_or_bus_format for the @layer, and return a pointer
+ * to the format descriptor. Search key choice depends on @layer mode, for live
+ * layers search is done by zynqmp_disp_format.bus_fmt, and for non-live layers
+ * zynqmp_disp_format.drm_fmt is used.
*
* Return: A pointer to the format descriptor if found, NULL otherwise
*/
static const struct zynqmp_disp_format *
zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
- u32 drm_fmt)
+ u32 drm_or_bus_format)
{
unsigned int i;
+ const struct zynqmp_disp_format *disp_format;
for (i = 0; i < layer->info->num_formats; i++) {
- if (layer->info->formats[i].drm_fmt == drm_fmt)
- return &layer->info->formats[i];
+ disp_format = &layer->info->formats[i];
+ if ((layer->mode == ZYNQMP_DPSUB_LAYER_LIVE &&
+ disp_format->bus_fmt == drm_or_bus_format) ||
+ (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE &&
+ disp_format->drm_fmt == drm_or_bus_format))
+ return disp_format;
}
return NULL;
@@ -992,20 +1009,25 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
/**
* zynqmp_disp_layer_set_format - Set the layer format
* @layer: The layer
- * @info: The format info
+ * @drm_or_bus_format: DRM or media bus format
*
* Set the format for @layer to @info. The layer must be disabled.
*/
void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
- const struct drm_format_info *info)
+ u32 drm_or_bus_format)
{
unsigned int i;
- layer->disp_fmt = zynqmp_disp_layer_find_format(layer, info->format);
- layer->drm_fmt = info;
+ layer->disp_fmt = zynqmp_disp_layer_find_format(layer, drm_or_bus_format);
+ if (WARN_ON(!layer->disp_fmt))
+ return;
zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
+ layer->drm_fmt = drm_format_info(layer->disp_fmt->drm_fmt);
+ if (!layer->drm_fmt)
+ return;
+
if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
return;
@@ -1013,7 +1035,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
* Set pconfig for each DMA channel to indicate they're part of a
* video group.
*/
- for (i = 0; i < info->num_planes; i++) {
+ for (i = 0; i < layer->drm_fmt->num_planes; i++) {
struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
struct xilinx_dpdma_peripheral_config pconfig = {
.video_group = true,
@@ -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);
@@ -1299,15 +1299,20 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
struct drm_bridge_state *old_bridge_state)
{
struct zynqmp_disp_layer *layer;
- const struct drm_format_info *info;
+ struct drm_bridge_state *bridge_state;
+ u32 bus_fmt;
layer = zynqmp_dp_disp_connected_live_layer(dp);
if (!layer)
return;
- /* TODO: Make the format configurable. */
- info = drm_format_info(DRM_FORMAT_YUV422);
- zynqmp_disp_layer_set_format(layer, info);
+ bridge_state = drm_atomic_get_new_bridge_state(old_bridge_state->base.state,
+ old_bridge_state->bridge);
+ if (WARN_ON(!bridge_state))
+ return;
+
+ bus_fmt = bridge_state->input_bus_cfg.format;
+ zynqmp_disp_layer_set_format(layer, bus_fmt);
zynqmp_disp_layer_enable(layer);
if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
@@ -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);