| Message ID | 20240226-dp-live-fmt-v1-3-b78c3f69c9d8@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Setting live video input format for ZynqMP DPSUB | expand |
Hi Anatoliy, Thank you for the patch. On Mon, Feb 26, 2024 at 08:44:44PM -0800, 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. > > Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com> > --- > drivers/gpu/drm/xlnx/zynqmp_disp.c | 52 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 ++ > drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++--- > drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 ++++++--- > 4 files changed, 67 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c > index ee99aad915ba..1c3ffdee6b8e 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c > @@ -416,6 +416,34 @@ static bool zynqmp_disp_layer_is_video(const struct zynqmp_disp_layer *layer) > return layer->id == ZYNQMP_DPSUB_LAYER_VID; > } > > +/** > + * zynqmp_disp_avbuf_set_live_format - Set live input format for a layer > + * @disp: Display controller > + * @layer: The layer > + * @fmt: The format information > + * > + * Set the live video input format for @layer to @fmt. > + */ > +static void zynqmp_disp_avbuf_set_live_format(struct zynqmp_disp *disp, > + struct zynqmp_disp_layer *layer, > + const struct zynqmp_disp_format *fmt) > +{ > + u32 reg, i; > + > + reg = zynqmp_disp_layer_is_video(layer) > + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG > + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG; > + zynqmp_disp_avbuf_write(disp, reg, fmt->buf_fmt); > + > + for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; ++i) { > + reg = zynqmp_disp_layer_is_video(layer) > + ? ZYNQMP_DISP_AV_BUF_LIVD_VID_COMP_SF(i) > + : ZYNQMP_DISP_AV_BUF_LIVD_GFX_COMP_SF(i); > + zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]); > + } This is identical to zynqmp_disp_avbuf_set_format(), you should avoid duplicating code. > + layer->disp_fmt = fmt; > +} > + > /** > * zynqmp_disp_avbuf_set_format - Set the input format for a layer > * @disp: Display controller > @@ -979,6 +1007,30 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer) > zynqmp_disp_blend_layer_disable(layer->disp, layer); > } > > +/** > + * zynqmp_disp_layer_set_live_format - Set live layer input format > + * @layer: The layer > + * @info: Input media bus format > + * > + * Set the live @layer input bus format. The layer must be disabled. > + */ > +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, > + u32 bus_format) I'd prefer reusing zynqmp_disp_layer_set_format(), and handling the differences between live and non-live input there. There's already a dma_enabled check in that function. > +{ > + int i; > + const struct zynqmp_disp_format *fmt; > + > + for (i = 0; i < ARRAY_SIZE(avbuf_live_fmts); ++i) { > + fmt = &avbuf_live_fmts[i]; > + if (fmt->bus_fmt == bus_format) { > + layer->disp_fmt = fmt; > + layer->drm_fmt = drm_format_info(fmt->drm_fmt); > + zynqmp_disp_avbuf_set_live_format(layer->disp, layer, fmt); > + return; > + } > + } > +} > + > /** > * zynqmp_disp_layer_set_format - Set the layer format > * @layer: The layer > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h > index c2c8dd4896ba..f244b7d2346a 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h > @@ -66,6 +66,8 @@ 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); > +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, > + u32 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_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) This change isn't even mentioned in the commit message. It should be split to a separate patch. > #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 > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 9cb7ac9f3097..0d5dffd20ad1 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 (bridge_state) { > + bus_fmt = bridge_state->input_bus_cfg.format; > + zynqmp_disp_layer_set_live_format(layer, bus_fmt); > + } else > + return; if (!bridge_state) return; bus_fmt = bridge_state->input_bus_cfg.format; zynqmp_disp_layer_set_live_format(layer, bus_fmt); But more importantly, why would this fail ? If it does something is seriously wrong and the display won't be working. I'd expect at least a warning, but you should instead ensure it never fails. > > - /* 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) >
Hi Laurent, Thanks a lot for the review. > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Wednesday, February 28, 2024 8:08 AM > To: Klymenko, Anatoliy <Anatoliy.Klymenko@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard > <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David > Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Simek, Michal > <michal.simek@amd.com>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil > Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Jonas > Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; dri- > devel@lists.freedesktop.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 3/4] 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 Mon, Feb 26, 2024 at 08:44:44PM -0800, 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. > > > > Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com> > > --- > > drivers/gpu/drm/xlnx/zynqmp_disp.c | 52 > +++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 ++ > > drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++--- > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 ++++++--- > > 4 files changed, 67 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c > > b/drivers/gpu/drm/xlnx/zynqmp_disp.c > > index ee99aad915ba..1c3ffdee6b8e 100644 > > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c > > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c > > @@ -416,6 +416,34 @@ static bool zynqmp_disp_layer_is_video(const struct > zynqmp_disp_layer *layer) > > return layer->id == ZYNQMP_DPSUB_LAYER_VID; } > > > > +/** > > + * zynqmp_disp_avbuf_set_live_format - Set live input format for a > > +layer > > + * @disp: Display controller > > + * @layer: The layer > > + * @fmt: The format information > > + * > > + * Set the live video input format for @layer to @fmt. > > + */ > > +static void zynqmp_disp_avbuf_set_live_format(struct zynqmp_disp *disp, > > + struct zynqmp_disp_layer *layer, > > + const struct > > +zynqmp_disp_format *fmt) { > > + u32 reg, i; > > + > > + reg = zynqmp_disp_layer_is_video(layer) > > + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG > > + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG; > > + zynqmp_disp_avbuf_write(disp, reg, fmt->buf_fmt); > > + > > + for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; ++i) { > > + reg = zynqmp_disp_layer_is_video(layer) > > + ? ZYNQMP_DISP_AV_BUF_LIVD_VID_COMP_SF(i) > > + : ZYNQMP_DISP_AV_BUF_LIVD_GFX_COMP_SF(i); > > + zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]); > > + } > > This is identical to zynqmp_disp_avbuf_set_format(), you should avoid duplicating > code. > Yeah, there are similarities - let me think on how to refactor this properly. > > + layer->disp_fmt = fmt; > > +} > > + > > /** > > * zynqmp_disp_avbuf_set_format - Set the input format for a layer > > * @disp: Display controller > > @@ -979,6 +1007,30 @@ void zynqmp_disp_layer_disable(struct > zynqmp_disp_layer *layer) > > zynqmp_disp_blend_layer_disable(layer->disp, layer); } > > > > +/** > > + * zynqmp_disp_layer_set_live_format - Set live layer input format > > + * @layer: The layer > > + * @info: Input media bus format > > + * > > + * Set the live @layer input bus format. The layer must be disabled. > > + */ > > +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, > > + u32 bus_format) > > I'd prefer reusing zynqmp_disp_layer_set_format(), and handling the differences > between live and non-live input there. There's already a dma_enabled check in > that function. > There is a difference between setting format for dma-backed layer vs live input layer. In the first case we have memory layout in fourcc format, but in the second case we have video signal described by media bus format. Anyways, let me check if I can unify both cases. > > +{ > > + int i; > > + const struct zynqmp_disp_format *fmt; > > + > > + for (i = 0; i < ARRAY_SIZE(avbuf_live_fmts); ++i) { > > + fmt = &avbuf_live_fmts[i]; > > + if (fmt->bus_fmt == bus_format) { > > + layer->disp_fmt = fmt; > > + layer->drm_fmt = drm_format_info(fmt->drm_fmt); > > + zynqmp_disp_avbuf_set_live_format(layer->disp, layer, fmt); > > + return; > > + } > > + } > > +} > > + > > /** > > * zynqmp_disp_layer_set_format - Set the layer format > > * @layer: The layer > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h > > b/drivers/gpu/drm/xlnx/zynqmp_disp.h > > index c2c8dd4896ba..f244b7d2346a 100644 > > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h > > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h > > @@ -66,6 +66,8 @@ 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); > > +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, > > + u32 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_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) > > This change isn't even mentioned in the commit message. It should be split to a > separate patch. > This patch is the first instance of these defines usage. Anyways, I'll split it. Thank you. > > #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 > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > index 9cb7ac9f3097..0d5dffd20ad1 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 (bridge_state) { > > + bus_fmt = bridge_state->input_bus_cfg.format; > > + zynqmp_disp_layer_set_live_format(layer, bus_fmt); > > + } else > > + return; > > if (!bridge_state) > return; > > bus_fmt = bridge_state->input_bus_cfg.format; > zynqmp_disp_layer_set_live_format(layer, bus_fmt); > > But more importantly, why would this fail ? If it does something is seriously wrong > and the display won't be working. I'd expect at least a warning, but you should > instead ensure it never fails. > According to drm_atomic_get_new_bridge_state() doc, it is possible that bridge state is not a part of the global atomic state. But, most likely in this case bridge enable() callback will be omitted completely. I'll check this - thank you. > > > > - /* 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) > > > > -- > Regards, > > Laurent Pinchart Thank you, Anatoliy
Hi Anatoliy, kernel test robot noticed the following build warnings: [auto build test WARNING on bfa4437fd3938ae2e186e7664b2db65bb8775670] url: https://github.com/intel-lab-lkp/linux/commits/Anatoliy-Klymenko/drm-xlnx-zynqmp_dpsub-Set-layer-mode-during-creation/20240227-124631 base: bfa4437fd3938ae2e186e7664b2db65bb8775670 patch link: https://lore.kernel.org/r/20240226-dp-live-fmt-v1-3-b78c3f69c9d8%40amd.com patch subject: [PATCH 3/4] drm: xlnx: zynqmp_dpsub: Set input live format config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20240304/202403040104.mwzqu5gs-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240304/202403040104.mwzqu5gs-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403040104.mwzqu5gs-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or struct member 'blend' not described in 'zynqmp_disp' drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or struct member 'avbuf' not described in 'zynqmp_disp' drivers/gpu/drm/xlnx/zynqmp_disp.c:164: warning: Function parameter or struct member 'audio' not described in 'zynqmp_disp' >> drivers/gpu/drm/xlnx/zynqmp_disp.c:1019: warning: Function parameter or struct member 'bus_format' not described in 'zynqmp_disp_layer_set_live_format' >> drivers/gpu/drm/xlnx/zynqmp_disp.c:1019: warning: Excess function parameter 'info' description in 'zynqmp_disp_layer_set_live_format' vim +1019 drivers/gpu/drm/xlnx/zynqmp_disp.c 1009 1010 /** 1011 * zynqmp_disp_layer_set_live_format - Set live layer input format 1012 * @layer: The layer 1013 * @info: Input media bus format 1014 * 1015 * Set the live @layer input bus format. The layer must be disabled. 1016 */ 1017 void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, 1018 u32 bus_format) > 1019 { 1020 int i; 1021 const struct zynqmp_disp_format *fmt; 1022 1023 for (i = 0; i < ARRAY_SIZE(avbuf_live_fmts); ++i) { 1024 fmt = &avbuf_live_fmts[i]; 1025 if (fmt->bus_fmt == bus_format) { 1026 layer->disp_fmt = fmt; 1027 layer->drm_fmt = drm_format_info(fmt->drm_fmt); 1028 zynqmp_disp_avbuf_set_live_format(layer->disp, layer, fmt); 1029 return; 1030 } 1031 } 1032 } 1033
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c index ee99aad915ba..1c3ffdee6b8e 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c @@ -416,6 +416,34 @@ static bool zynqmp_disp_layer_is_video(const struct zynqmp_disp_layer *layer) return layer->id == ZYNQMP_DPSUB_LAYER_VID; } +/** + * zynqmp_disp_avbuf_set_live_format - Set live input format for a layer + * @disp: Display controller + * @layer: The layer + * @fmt: The format information + * + * Set the live video input format for @layer to @fmt. + */ +static void zynqmp_disp_avbuf_set_live_format(struct zynqmp_disp *disp, + struct zynqmp_disp_layer *layer, + const struct zynqmp_disp_format *fmt) +{ + u32 reg, i; + + reg = zynqmp_disp_layer_is_video(layer) + ? ZYNQMP_DISP_AV_BUF_LIVE_VID_CONFIG + : ZYNQMP_DISP_AV_BUF_LIVE_GFX_CONFIG; + zynqmp_disp_avbuf_write(disp, reg, fmt->buf_fmt); + + for (i = 0; i < ZYNQMP_DISP_AV_BUF_NUM_SF; ++i) { + reg = zynqmp_disp_layer_is_video(layer) + ? ZYNQMP_DISP_AV_BUF_LIVD_VID_COMP_SF(i) + : ZYNQMP_DISP_AV_BUF_LIVD_GFX_COMP_SF(i); + zynqmp_disp_avbuf_write(disp, reg, fmt->sf[i]); + } + layer->disp_fmt = fmt; +} + /** * zynqmp_disp_avbuf_set_format - Set the input format for a layer * @disp: Display controller @@ -979,6 +1007,30 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer) zynqmp_disp_blend_layer_disable(layer->disp, layer); } +/** + * zynqmp_disp_layer_set_live_format - Set live layer input format + * @layer: The layer + * @info: Input media bus format + * + * Set the live @layer input bus format. The layer must be disabled. + */ +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, + u32 bus_format) +{ + int i; + const struct zynqmp_disp_format *fmt; + + for (i = 0; i < ARRAY_SIZE(avbuf_live_fmts); ++i) { + fmt = &avbuf_live_fmts[i]; + if (fmt->bus_fmt == bus_format) { + layer->disp_fmt = fmt; + layer->drm_fmt = drm_format_info(fmt->drm_fmt); + zynqmp_disp_avbuf_set_live_format(layer->disp, layer, fmt); + return; + } + } +} + /** * zynqmp_disp_layer_set_format - Set the layer format * @layer: The layer diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h index c2c8dd4896ba..f244b7d2346a 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h @@ -66,6 +66,8 @@ 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); +void zynqmp_disp_layer_set_live_format(struct zynqmp_disp_layer *layer, + u32 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_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 diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index 9cb7ac9f3097..0d5dffd20ad1 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 (bridge_state) { + bus_fmt = bridge_state->input_bus_cfg.format; + zynqmp_disp_layer_set_live_format(layer, bus_fmt); + } else + return; - /* 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)
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. Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com> --- drivers/gpu/drm/xlnx/zynqmp_disp.c | 52 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/xlnx/zynqmp_disp.h | 2 ++ drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++--- drivers/gpu/drm/xlnx/zynqmp_dp.c | 13 ++++++--- 4 files changed, 67 insertions(+), 8 deletions(-)