On Mon, Oct 26, 2026 at 06:05:12PM +0800, Liu Ying wrote:
> Display Prefetch Resolve Gasket(PRG) is a part of a FetchUnit's
> prefetch engine. It sits between a Display Prefetch Resolve
> Channel(DPRC) and a FetchUnit. Add a platform driver to support
> the PRG.
>
> Reviewed-by: Frank Li <[email protected]>
> Acked-by: Maxime Ripard <[email protected]>
> Signed-off-by: Liu Ying <[email protected]>
> ---
> v7:
> - Collect Maxime's A-b tag.
> - Return 128B burst size from dc_prg_burst_size_fixup() when baddr == 0.
> This avoids undefined behavior of __ffs(0). (Sashiko AI bot)
> - Initialize and declare prg_node at the same time. (Sashiko AI bot)
> - Add device link between PRG's consumer and PRG. (Sashiko AI bot)
>
> v6:
> - Include "dc-drv.h" to avoid build warning with C=1.
> - Fix PRG BYPASS programming.
>
> v4:
> - Sort bits and bitfields in descending order for register PRG_CTRL.
>
> v2:
> - Manage clocks with bulk interfaces. (Frank)
> - Collect Frank's R-b tag.
> ---
> drivers/gpu/drm/imx/dc/Makefile | 2 +-
> drivers/gpu/drm/imx/dc/dc-drv.c | 1 +
> drivers/gpu/drm/imx/dc/dc-drv.h | 1 +
> drivers/gpu/drm/imx/dc/dc-prg.c | 314
> ++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/imx/dc/dc-prg.h | 35 +++++
> 5 files changed, 352 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/dc/Makefile b/drivers/gpu/drm/imx/dc/Makefile
> index b9d33c074984..e3a06ee3ce1a 100644
> --- a/drivers/gpu/drm/imx/dc/Makefile
> +++ b/drivers/gpu/drm/imx/dc/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> imx8-dc-drm-objs := dc-cf.o dc-crtc.o dc-de.o dc-drv.o dc-ed.o dc-fg.o
> dc-fl.o \
> - dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o \
> + dc-fu.o dc-fw.o dc-ic.o dc-kms.o dc-lb.o dc-pe.o dc-prg.o \
> dc-plane.o dc-tc.o
>
> obj-$(CONFIG_DRM_IMX8_DC) += imx8-dc-drm.o
> diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
> index 09387d62cc0b..1d01f896e427 100644
> --- a/drivers/gpu/drm/imx/dc/dc-drv.c
> +++ b/drivers/gpu/drm/imx/dc/dc-drv.c
> @@ -351,6 +351,7 @@ static struct platform_driver * const dc_drivers[] = {
> &dc_ic_driver,
> &dc_lb_driver,
> &dc_pe_driver,
> + &dc_prg_driver,
> &dc_tc_driver,
> &dc_driver,
> };
> diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
> index 1ec9304329eb..27ef2c552549 100644
> --- a/drivers/gpu/drm/imx/dc/dc-drv.h
> +++ b/drivers/gpu/drm/imx/dc/dc-drv.h
> @@ -81,6 +81,7 @@ extern struct platform_driver dc_fw_driver;
> extern struct platform_driver dc_ic_driver;
> extern struct platform_driver dc_lb_driver;
> extern struct platform_driver dc_pe_driver;
> +extern struct platform_driver dc_prg_driver;
> extern struct platform_driver dc_tc_driver;
>
> static inline int dc_subdev_get_id(const struct dc_subdev_info *info,
> diff --git a/drivers/gpu/drm/imx/dc/dc-prg.c b/drivers/gpu/drm/imx/dc/dc-prg.c
> new file mode 100644
> index 000000000000..43df5683b24d
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dc/dc-prg.c
> @@ -0,0 +1,314 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
This header can be removed.
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
This one too.
[...]
> +
> +void dc_prg_disable(struct dc_prg *prg)
> +{
> + regmap_write(prg->reg, PRG_CTRL, BYPASS);
> +
> + pm_runtime_put(prg->dev);
I have a question about this. I see Sashiko also flagged an issue regarding the
unconditional put when the pm_runtime_resume_and_get() fails in
dc_prg_configure(). The resume is, probably, unlikely to fail. But, I see a
situation where the dc_prg_configure() is never called: when the plane is
invisible. The dc_prg_disable() would still be called in flush() when the plane
is disabled, would it not?
> +}
> +
> +void dc_prg_disable_at_boot(struct dc_prg *prg)
> +{
> + regmap_write(prg->reg, PRG_CTRL, BYPASS);
> +
> + clk_bulk_disable_unprepare(prg->num_clks, prg->clks);
> +}
> +
> +static unsigned int dc_prg_burst_size_fixup(dma_addr_t baddr)
> +{
> + unsigned int burst_size;
> +
> + if (baddr == 0)
> + return 128U;
> +
> + burst_size = 1 << __ffs(baddr);
> + burst_size = round_up(burst_size, 8);
> + burst_size = min(burst_size, 128U);
> +
> + return burst_size;
> +}
> +
> +static unsigned int
> +dc_prg_stride_fixup(unsigned int stride, unsigned int burst_size)
> +{
> + return round_up(stride, burst_size);
> +}
> +
> +void dc_prg_configure(struct dc_prg *prg,
> + unsigned int width, unsigned int height,
> + unsigned int stride, unsigned int bits_per_pixel,
> + dma_addr_t baddr, bool start)
> +{
> + struct device *dev = prg->dev;
> + unsigned int burst_size;
> + u32 val;
> + int ret;
> +
> + if (start) {
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to get RPM: %d\n", ret);
> + return;
> + }
> + }
> +
> + burst_size = dc_prg_burst_size_fixup(baddr);
> +
> + stride = dc_prg_stride_fixup(stride, burst_size);
> +
> + regmap_write(prg->reg, PRG_STRIDE, STRIDE(stride));
> + regmap_write(prg->reg, PRG_WIDTH, WIDTH(width));
> + regmap_write(prg->reg, PRG_HEIGHT, HEIGHT(height));
> + regmap_write(prg->reg, PRG_OFFSET, 0);
> + regmap_write(prg->reg, PRG_BADDR, baddr);
> +
> + val = SHADOW_LOAD_MODE | SC_DATA_TYPE_8BIT | HANDSHAKE_MODE_4LINES;
> +
> + switch (bits_per_pixel) {
> + case 32:
> + val |= DES_DATA_TYPE_32BPP;
> + break;
> + case 24:
> + val |= DES_DATA_TYPE_24BPP;
> + break;
> + case 16:
> + val |= DES_DATA_TYPE_16BPP;
> + break;
> + case 8:
> + val |= DES_DATA_TYPE_8BPP;
> + break;
> + }
> +
> + /* no shadow for the first frame */
> + val |= start ? BYPASS : SHADOW_EN;
> +
> + regmap_write(prg->reg, PRG_CTRL, val);
> +}
> +
> +void dc_prg_reg_update(struct dc_prg *prg)
> +{
> + regmap_write(prg->reg, PRG_REG_UPDATE, REG_UPDATE);
> +}
> +
> +void dc_prg_shadow_enable(struct dc_prg *prg)
> +{
> + regmap_write(prg->reg, PRG_CTRL + SET, SHADOW_EN);
> +}
> +
> +bool dc_prg_stride_supported(struct dc_prg *prg,
It looks like you're not using prg. Can it be removed?
--
Thanks,
Laurentiu
