diff mbox series

[5/5] drm/msm/dp: Allow sub-regions to be specified in DT

Message ID 20210722024227.3313096-6-bjorn.andersson@linaro.org
State Superseded
Headers show
Series drm/msm/dp: Allow variation in register regions | expand

Commit Message

Bjorn Andersson July 22, 2021, 2:42 a.m. UTC
Not all platforms has P0 at an offset of 0x1000 from the base address,
so add support for specifying each sub-region in DT. The code falls back
to the predefined offsets in the case that only a single reg is
specified, in order to support existing DT.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_parser.c | 49 +++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 11 deletions(-)

Comments

Abhinav Kumar July 23, 2021, 8:33 p.m. UTC | #1
On 2021-07-21 19:42, Bjorn Andersson wrote:
> Not all platforms has P0 at an offset of 0x1000 from the base address,

> so add support for specifying each sub-region in DT. The code falls 

> back

> to the predefined offsets in the case that only a single reg is

> specified, in order to support existing DT.

> 

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

The change itself looks good, hence
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>


But as a follow up to this change, can we move the existing DP DT on
sc7180 to this model and then drop this legacy path?

That will clean this up nicely.

> ---

>  drivers/gpu/drm/msm/dp/dp_parser.c | 49 +++++++++++++++++++++++-------

>  1 file changed, 38 insertions(+), 11 deletions(-)

> 

> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c

> b/drivers/gpu/drm/msm/dp/dp_parser.c

> index 1a10901ae574..fc8a6452f641 100644

> --- a/drivers/gpu/drm/msm/dp/dp_parser.c

> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c

> @@ -63,18 +63,45 @@ static int dp_parser_ctrl_res(struct dp_parser 

> *parser)

>  		return PTR_ERR(dss->ahb);

>  	}

> 

> -	if (dss->ahb_len < DP_DEFAULT_P0_OFFSET + DP_DEFAULT_P0_SIZE) {

> -		DRM_ERROR("legacy memory region not large enough\n");

> -		return -EINVAL;

> -	}

> +	dss->aux = dp_ioremap(pdev, 1, &dss->aux_len);

> +	if (IS_ERR(dss->aux)) {

> +		/*

> +		 * The initial binding had a single reg, but in order to

> +		 * support variation in the sub-region sizes this was split.

> +		 * dp_ioremap() will fail with -ENODEV here if only a single

> +		 * reg is specified, so fill in the sub-region offsets and

> +		 * lengths based on this single region.

> +		 */

> +		if (PTR_ERR(dss->aux) == -ENODEV) {

> +			if (dss->ahb_len < DP_DEFAULT_P0_OFFSET + DP_DEFAULT_P0_SIZE) {

> +				DRM_ERROR("legacy memory region not large enough\n");

> +				return -EINVAL;

> +			}

> +

> +			dss->ahb_len = DP_DEFAULT_AHB_SIZE;

> +			dss->aux = dss->ahb + DP_DEFAULT_AUX_OFFSET;

> +			dss->aux_len = DP_DEFAULT_AUX_SIZE;

> +			dss->link = dss->ahb + DP_DEFAULT_LINK_OFFSET;

> +			dss->link_len = DP_DEFAULT_LINK_SIZE;

> +			dss->p0 = dss->ahb + DP_DEFAULT_P0_OFFSET;

> +			dss->p0_len = DP_DEFAULT_P0_SIZE;

> +		} else {

> +			DRM_ERROR("unable to remap aux region: %pe\n", dss->aux);

> +			return PTR_ERR(dss->aux);

> +		}

> +	} else {

> +		dss->link = dp_ioremap(pdev, 2, &dss->link_len);

> +		if (IS_ERR(dss->link)) {

> +			DRM_ERROR("unable to remap link region: %pe\n", dss->link);

> +			return PTR_ERR(dss->link);

> +		}

> 

> -	dss->ahb_len = DP_DEFAULT_AHB_SIZE;

> -	dss->aux = dss->ahb + DP_DEFAULT_AUX_OFFSET;

> -	dss->aux_len = DP_DEFAULT_AUX_SIZE;

> -	dss->link = dss->ahb + DP_DEFAULT_LINK_OFFSET;

> -	dss->link_len = DP_DEFAULT_LINK_SIZE;

> -	dss->p0 = dss->ahb + DP_DEFAULT_P0_OFFSET;

> -	dss->p0_len = DP_DEFAULT_P0_SIZE;

> +		dss->p0 = dp_ioremap(pdev, 3, &dss->p0_len);

> +		if (IS_ERR(dss->p0)) {

> +			DRM_ERROR("unable to remap p0 region: %pe\n", dss->p0);

> +			return PTR_ERR(dss->p0);

> +		}

> +	}

> 

>  	io->phy = devm_phy_get(&pdev->dev, "dp");

>  	if (IS_ERR(io->phy))
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 1a10901ae574..fc8a6452f641 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -63,18 +63,45 @@  static int dp_parser_ctrl_res(struct dp_parser *parser)
 		return PTR_ERR(dss->ahb);
 	}
 
-	if (dss->ahb_len < DP_DEFAULT_P0_OFFSET + DP_DEFAULT_P0_SIZE) {
-		DRM_ERROR("legacy memory region not large enough\n");
-		return -EINVAL;
-	}
+	dss->aux = dp_ioremap(pdev, 1, &dss->aux_len);
+	if (IS_ERR(dss->aux)) {
+		/*
+		 * The initial binding had a single reg, but in order to
+		 * support variation in the sub-region sizes this was split.
+		 * dp_ioremap() will fail with -ENODEV here if only a single
+		 * reg is specified, so fill in the sub-region offsets and
+		 * lengths based on this single region.
+		 */
+		if (PTR_ERR(dss->aux) == -ENODEV) {
+			if (dss->ahb_len < DP_DEFAULT_P0_OFFSET + DP_DEFAULT_P0_SIZE) {
+				DRM_ERROR("legacy memory region not large enough\n");
+				return -EINVAL;
+			}
+
+			dss->ahb_len = DP_DEFAULT_AHB_SIZE;
+			dss->aux = dss->ahb + DP_DEFAULT_AUX_OFFSET;
+			dss->aux_len = DP_DEFAULT_AUX_SIZE;
+			dss->link = dss->ahb + DP_DEFAULT_LINK_OFFSET;
+			dss->link_len = DP_DEFAULT_LINK_SIZE;
+			dss->p0 = dss->ahb + DP_DEFAULT_P0_OFFSET;
+			dss->p0_len = DP_DEFAULT_P0_SIZE;
+		} else {
+			DRM_ERROR("unable to remap aux region: %pe\n", dss->aux);
+			return PTR_ERR(dss->aux);
+		}
+	} else {
+		dss->link = dp_ioremap(pdev, 2, &dss->link_len);
+		if (IS_ERR(dss->link)) {
+			DRM_ERROR("unable to remap link region: %pe\n", dss->link);
+			return PTR_ERR(dss->link);
+		}
 
-	dss->ahb_len = DP_DEFAULT_AHB_SIZE;
-	dss->aux = dss->ahb + DP_DEFAULT_AUX_OFFSET;
-	dss->aux_len = DP_DEFAULT_AUX_SIZE;
-	dss->link = dss->ahb + DP_DEFAULT_LINK_OFFSET;
-	dss->link_len = DP_DEFAULT_LINK_SIZE;
-	dss->p0 = dss->ahb + DP_DEFAULT_P0_OFFSET;
-	dss->p0_len = DP_DEFAULT_P0_SIZE;
+		dss->p0 = dp_ioremap(pdev, 3, &dss->p0_len);
+		if (IS_ERR(dss->p0)) {
+			DRM_ERROR("unable to remap p0 region: %pe\n", dss->p0);
+			return PTR_ERR(dss->p0);
+		}
+	}
 
 	io->phy = devm_phy_get(&pdev->dev, "dp");
 	if (IS_ERR(io->phy))