Re: [PATCH v6 1/8] drm/ingenic: prepare ingenic drm for later addition of JZ4780

Paul Cercueil
2021-11-16T01:22:53Z

Hi Nikolaus,

I will look at the patches in depth in the coming days.

Le mer., nov. 10 2021 at 20:43:26 +0100, H. Nikolaus Schaller
<hns@goldelico.com> a écrit :
> This changes the way the regmap is allocated to prepare for the
> later addition of the JZ4780 which has more registers and bits
> than the others.
>
> Therefore we make the regmap as big as the reg property in
> the device tree tells.
>
> Suggested-by: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 462bc0f35f1bf..4abfe5b094174 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -173,7 +173,6 @@ static const struct regmap_config
> ingenic_drm_regmap_config = {
> .val_bits = 32,
> .reg_stride = 4,
>
> - .max_register = JZ_REG_LCD_SIZE1,
> .writeable_reg = ingenic_drm_writeable_reg,
> };
>
> @@ -1011,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev,
> bool has_components)
> struct ingenic_drm_bridge *ib;
> struct drm_device *drm;
> void __iomem *base;
> + struct resource *res;
> + struct regmap_config regmap_config;
> long parent_rate;
> unsigned int i, clone_mask = 0;
> int ret, irq;
> @@ -1056,14 +1057,16 @@ static int ingenic_drm_bind(struct device
> *dev, bool has_components)
> drm->mode_config.funcs = &ingenic_drm_mode_config_funcs;
> drm->mode_config.helper_private = &ingenic_drm_mode_config_helpers;
>
> - base = devm_platform_ioremap_resource(pdev, 0);
> + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> if (IS_ERR(base)) {
> dev_err(dev, "Failed to get memory resource\n");
> return PTR_ERR(base);
> }
>
> + regmap_config = ingenic_drm_regmap_config;
> + regmap_config.max_register = res->end - res->start - 4;

Just a quick feedback here: I just tested and it's actually just
(res->end - res->start), otherwise the last register of the memory area
set in DT is inaccessible.

Cheers,
-Paul

> priv->map = devm_regmap_init_mmio(dev, base,
> - &ingenic_drm_regmap_config);
> + &regmap_config);
> if (IS_ERR(priv->map)) {
> dev_err(dev, "Failed to create regmap\n");
> return PTR_ERR(priv->map);
> --
> 2.33.0
>