On Fri 28 May 18:40 CDT 2021, abhinavk@xxxxxxxxxxxxxx wrote: > On 2021-05-10 21:20, Bjorn Andersson wrote: > > The eDP controller found in SC8180x is at large compatible with the > > current implementation, but has its register blocks at slightly > > different offsets. > > > > Add the compatible and the new register layout. > > > I am not able to completely recall the history of why in the DP bindings > we added DP register base as a big hunk and let catalog handle the submodule > offsets. > > I guess earlier that made sense because DP sub-block offsets were fixed. > But if we plan to re-use the DP driver for eDP as well like this series, > then maybe it might be > better if this comes from device tree like the earlier version was planning > to > > https://patchwork.kernel.org/project/dri-devel/patch/0101016ec6ddf446-e87ab1ce-5cbf-40a0-a0bb-cd0151cd577a-000000@xxxxxxxxxxxxxxxxxxxxxxx/ > > > +- reg: Base address and length of DP hardware's memory > mapped regions. > +- cell-index: Specifies the controller instance. > +- reg-names: A list of strings that name the list of regs. > + "dp_ahb" - DP controller memory region. > + "dp_aux" - DP AUX memory region. > + "dp_link" - DP link layer memory region. > + "dp_p0" - DP pixel clock domain memory region. > + "dp_phy" - DP PHY memory region. > + "dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region. > + "dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region. > > Now there is more reason to separate the sub-module offsets like > ahb/aux/link/p0 I like it, will rewrite the patch accordingly. Regards, Bjorn > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 1 + > > drivers/gpu/drm/msm/dp/dp_parser.c | 28 ++++++++++++++++++++-------- > > 2 files changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index d1319b58e901..0be03bdc882c 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > @@ -121,6 +121,7 @@ struct dp_display_private { > > > > static const struct of_device_id dp_dt_match[] = { > > {.compatible = "qcom,sc7180-dp"}, > > + { .compatible = "qcom,sc8180x-edp" }, > > {} > > }; > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c > > b/drivers/gpu/drm/msm/dp/dp_parser.c > > index 51ec85b4803b..47cf18bba4b2 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_parser.c > > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c > > @@ -251,6 +251,7 @@ static int dp_parser_clock(struct dp_parser *parser) > > static int dp_parser_parse(struct dp_parser *parser) > > { > > struct dss_io_data *io = &parser->io.dp_controller; > > + struct device *dev = &parser->pdev->dev; > > int rc = 0; > > > > if (!parser) { > > @@ -276,14 +277,25 @@ static int dp_parser_parse(struct dp_parser > > *parser) > > */ > > parser->regulator_cfg = &sdm845_dp_reg_cfg; > > > > - io->ahb = io->base + 0x0; > > - io->ahb_len = 0x200; > > - io->aux = io->base + 0x200; > > - io->aux_len = 0x200; > > - io->link = io->base + 0x400; > > - io->link_len = 0x600; > > - io->p0 = io->base + 0x1000; > > - io->p0_len = 0x400; > > + if (of_device_is_compatible(dev->of_node, "qcom,sc8180x-edp")) { > > + io->ahb = io->base + 0x0; > > + io->ahb_len = 0x200; > > + io->aux = io->base + 0x200; > > + io->aux_len = 0x200; > > + io->link = io->base + 0x400; > > + io->link_len = 0x600; > > + io->p0 = io->base + 0xa00; > > + io->p0_len = 0x400; > > + } else { > > + io->ahb = io->base + 0x0; > > + io->ahb_len = 0x200; > > + io->aux = io->base + 0x200; > > + io->aux_len = 0x200; > > + io->link = io->base + 0x400; > > + io->link_len = 0x600; > > + io->p0 = io->base + 0x1000; > > + io->p0_len = 0x400; > > + } > > > > return 0; > > }