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

[Paul Cercueil <paul@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> ---
>  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