It turns out that sc8180x (among others) doesn't have the same internal
layout of the 4 subblocks. This series therefor modifies the binding to
require all four regions to be described individually and then extends
the driver to read these four regions. The driver will fall back to read
the old single-reg format and apply the original offsets and sizes.
Bjorn Andersson (5):
dt-bindings: msm/dp: Change reg definition
drm/msm/dp: Use devres for ioremap()
drm/msm/dp: Refactor ioremap wrapper
drm/msm/dp: Store each subblock in the io region
drm/msm/dp: Allow sub-regions to be specified in DT
.../bindings/display/msm/dp-controller.yaml | 13 ++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 64 ++++-------
drivers/gpu/drm/msm/dp/dp_parser.c | 102 ++++++++++--------
drivers/gpu/drm/msm/dp/dp_parser.h | 11 +-
4 files changed, 100 insertions(+), 90 deletions(-)
--
2.29.2
reg was defined as one region covering the entire DP block, but the
memory map is actually split in 4 regions and obviously the size of
these regions differs between platforms.
Switch the reg to require that all four regions are specified instead.
It is expected that the implementation will handle existing DTBs, even
though the schema defines the new layout.
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v2:
- None
.../bindings/display/msm/dp-controller.yaml | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index d89b3c510c27..6bb424c21340 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -19,7 +19,12 @@ properties:
- qcom,sc7180-dp
reg:
- maxItems: 1
+ items:
+ - description: ahb register block
+ - description: aux register block
+ - description: link register block
+ - description: p0 register block
+ - description: p1 register block
interrupts:
maxItems: 1
@@ -99,7 +104,11 @@ examples:
displayport-controller@ae90000 {
compatible = "qcom,sc7180-dp";
- reg = <0xae90000 0x1400>;
+ reg = <0xae90000 0x200>,
+ <0xae90200 0x200>,
+ <0xae90400 0xc00>,
+ <0xae91000 0x400>,
+ <0xae91400 0x400>;
interrupt-parent = <&mdss>;
interrupts = <12>;
clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
--
2.29.2
In order to deal with multiple memory ranges in the following commit
change the ioremap wrapper to not poke directly into the dss_io_data
struct.
While at it, devm_ioremap_resource() already prints useful error
messages on failure, so omit the unnecessary prints from the caller.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v2:
- Switched to devm_platform_get_and_ioremap_resource()
drivers/gpu/drm/msm/dp/dp_parser.c | 35 ++++++++++--------------------
drivers/gpu/drm/msm/dp/dp_parser.h | 2 +-
2 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index c064ced78278..c05ba1990218 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -19,40 +19,27 @@ static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
},
};
-static int msm_dss_ioremap(struct platform_device *pdev,
- struct dss_io_data *io_data)
+static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t *len)
{
- struct resource *res = NULL;
+ struct resource *res;
+ void __iomem *base;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- DRM_ERROR("%pS->%s: msm_dss_get_res failed\n",
- __builtin_return_address(0), __func__);
- return -ENODEV;
- }
-
- io_data->len = (u32)resource_size(res);
- io_data->base = devm_ioremap(&pdev->dev, res->start, io_data->len);
- if (!io_data->base) {
- DRM_ERROR("%pS->%s: ioremap failed\n",
- __builtin_return_address(0), __func__);
- return -EIO;
- }
+ base = devm_platform_get_and_ioremap_resource(pdev, idx, &res);
+ if (!IS_ERR(base))
+ *len = resource_size(res);
- return 0;
+ return base;
}
static int dp_parser_ctrl_res(struct dp_parser *parser)
{
- int rc = 0;
struct platform_device *pdev = parser->pdev;
struct dp_io *io = &parser->io;
+ struct dss_io_data *dss = &io->dp_controller;
- rc = msm_dss_ioremap(pdev, &io->dp_controller);
- if (rc) {
- DRM_ERROR("unable to remap dp io resources, rc=%d\n", rc);
- return rc;
- }
+ dss->base = dp_ioremap(pdev, 0, &dss->len);
+ if (IS_ERR(dss->base))
+ return PTR_ERR(dss->base);
io->phy = devm_phy_get(&pdev->dev, "dp");
if (IS_ERR(io->phy))
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 34b49628bbaf..dc62e70b1640 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -26,7 +26,7 @@ enum dp_pm_type {
};
struct dss_io_data {
- u32 len;
+ size_t len;
void __iomem *base;
};
--
2.29.2
The non-devres version of ioremap is used, which requires manual
cleanup. But the code paths leading here is mixed with other devres
users, so rely on this for ioremap as well to simplify the code.
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v2:
- None
drivers/gpu/drm/msm/dp/dp_parser.c | 29 ++++-------------------------
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index 0519dd3ac3c3..c064ced78278 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -32,7 +32,7 @@ static int msm_dss_ioremap(struct platform_device *pdev,
}
io_data->len = (u32)resource_size(res);
- io_data->base = ioremap(res->start, io_data->len);
+ io_data->base = devm_ioremap(&pdev->dev, res->start, io_data->len);
if (!io_data->base) {
DRM_ERROR("%pS->%s: ioremap failed\n",
__builtin_return_address(0), __func__);
@@ -42,22 +42,6 @@ static int msm_dss_ioremap(struct platform_device *pdev,
return 0;
}
-static void msm_dss_iounmap(struct dss_io_data *io_data)
-{
- if (io_data->base) {
- iounmap(io_data->base);
- io_data->base = NULL;
- }
- io_data->len = 0;
-}
-
-static void dp_parser_unmap_io_resources(struct dp_parser *parser)
-{
- struct dp_io *io = &parser->io;
-
- msm_dss_iounmap(&io->dp_controller);
-}
-
static int dp_parser_ctrl_res(struct dp_parser *parser)
{
int rc = 0;
@@ -67,19 +51,14 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
rc = msm_dss_ioremap(pdev, &io->dp_controller);
if (rc) {
DRM_ERROR("unable to remap dp io resources, rc=%d\n", rc);
- goto err;
+ return rc;
}
io->phy = devm_phy_get(&pdev->dev, "dp");
- if (IS_ERR(io->phy)) {
- rc = PTR_ERR(io->phy);
- goto err;
- }
+ if (IS_ERR(io->phy))
+ return PTR_ERR(io->phy);
return 0;
-err:
- dp_parser_unmap_io_resources(parser);
- return rc;
}
static int dp_parser_misc(struct dp_parser *parser)
--
2.29.2
Quoting Bjorn Andersson (2021-10-01 10:43:58)
> In order to deal with multiple memory ranges in the following commit
> change the ioremap wrapper to not poke directly into the dss_io_data
> struct.
>
> While at it, devm_ioremap_resource() already prints useful error
> messages on failure, so omit the unnecessary prints from the caller.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
I realize this will cause some prints when we use old DTs. I suppose
that's OK though because we'll have more incentive to update existing
DT.
On Mon 04 Oct 18:04 PDT 2021, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2021-10-01 10:43:58)
> > In order to deal with multiple memory ranges in the following commit
> > change the ioremap wrapper to not poke directly into the dss_io_data
> > struct.
> >
> > While at it, devm_ioremap_resource() already prints useful error
> > messages on failure, so omit the unnecessary prints from the caller.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
>
> Reviewed-by: Stephen Boyd <[email protected]>
>
> I realize this will cause some prints when we use old DTs. I suppose
> that's OK though because we'll have more incentive to update existing
> DT.
The use of the current binding is fairly limited, so I think that makes
sense. Abhinav also requested earlier that we do that and drop the
fallback sooner rather than later, which I would like to see as well.
Thanks,
Bjorn
On 2021-10-01 10:43, Bjorn Andersson wrote:
> reg was defined as one region covering the entire DP block, but the
> memory map is actually split in 4 regions and obviously the size of
> these regions differs between platforms.
>
> Switch the reg to require that all four regions are specified instead.
> It is expected that the implementation will handle existing DTBs, even
> though the schema defines the new layout.
>
> Reviewed-by: Stephen Boyd <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
> ---
>
> Changes since v2:
> - None
>
> .../bindings/display/msm/dp-controller.yaml | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index d89b3c510c27..6bb424c21340 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -19,7 +19,12 @@ properties:
> - qcom,sc7180-dp
>
> reg:
> - maxItems: 1
> + items:
> + - description: ahb register block
> + - description: aux register block
> + - description: link register block
> + - description: p0 register block
> + - description: p1 register block
>
> interrupts:
> maxItems: 1
> @@ -99,7 +104,11 @@ examples:
>
> displayport-controller@ae90000 {
> compatible = "qcom,sc7180-dp";
> - reg = <0xae90000 0x1400>;
> + reg = <0xae90000 0x200>,
> + <0xae90200 0x200>,
> + <0xae90400 0xc00>,
> + <0xae91000 0x400>,
> + <0xae91400 0x400>;
> interrupt-parent = <&mdss>;
> interrupts = <12>;
> clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
On 2021-10-01 10:43, Bjorn Andersson wrote:
> In order to deal with multiple memory ranges in the following commit
> change the ioremap wrapper to not poke directly into the dss_io_data
> struct.
>
> While at it, devm_ioremap_resource() already prints useful error
> messages on failure, so omit the unnecessary prints from the caller.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
> ---
>
> Changes since v2:
> - Switched to devm_platform_get_and_ioremap_resource()
>
> drivers/gpu/drm/msm/dp/dp_parser.c | 35 ++++++++++--------------------
> drivers/gpu/drm/msm/dp/dp_parser.h | 2 +-
> 2 files changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> index c064ced78278..c05ba1990218 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -19,40 +19,27 @@ static const struct dp_regulator_cfg
> sdm845_dp_reg_cfg = {
> },
> };
>
> -static int msm_dss_ioremap(struct platform_device *pdev,
> - struct dss_io_data *io_data)
> +static void __iomem *dp_ioremap(struct platform_device *pdev, int
> idx, size_t *len)
> {
> - struct resource *res = NULL;
> + struct resource *res;
> + void __iomem *base;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - DRM_ERROR("%pS->%s: msm_dss_get_res failed\n",
> - __builtin_return_address(0), __func__);
> - return -ENODEV;
> - }
> -
> - io_data->len = (u32)resource_size(res);
> - io_data->base = devm_ioremap(&pdev->dev, res->start, io_data->len);
> - if (!io_data->base) {
> - DRM_ERROR("%pS->%s: ioremap failed\n",
> - __builtin_return_address(0), __func__);
> - return -EIO;
> - }
> + base = devm_platform_get_and_ioremap_resource(pdev, idx, &res);
> + if (!IS_ERR(base))
> + *len = resource_size(res);
>
> - return 0;
> + return base;
> }
>
> static int dp_parser_ctrl_res(struct dp_parser *parser)
> {
> - int rc = 0;
> struct platform_device *pdev = parser->pdev;
> struct dp_io *io = &parser->io;
> + struct dss_io_data *dss = &io->dp_controller;
>
> - rc = msm_dss_ioremap(pdev, &io->dp_controller);
> - if (rc) {
> - DRM_ERROR("unable to remap dp io resources, rc=%d\n", rc);
> - return rc;
> - }
> + dss->base = dp_ioremap(pdev, 0, &dss->len);
> + if (IS_ERR(dss->base))
> + return PTR_ERR(dss->base);
>
> io->phy = devm_phy_get(&pdev->dev, "dp");
> if (IS_ERR(io->phy))
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
> b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 34b49628bbaf..dc62e70b1640 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -26,7 +26,7 @@ enum dp_pm_type {
> };
>
> struct dss_io_data {
> - u32 len;
> + size_t len;
> void __iomem *base;
> };