[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YLJ1Yfg8jlrxILTE@builder.lan>
Date: Sat, 29 May 2021 12:09:53 -0500
From: Bjorn Andersson <bjorn.andersson@...aro.org>
To: abhinavk@...eaurora.org
Cc: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Stephen Boyd <swboyd@...omium.org>, sbillaka@...eaurora.org,
Tanmay Shah <tanmay@...eaurora.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
freedreno@...ts.freedesktop.org,
Chandan Uddaraju <chandanu@...eaurora.org>
Subject: Re: [Freedreno] [PATCH 4/4] drm/msm/dp: Add support for SC8180x eDP
On Fri 28 May 18:40 CDT 2021, abhinavk@...eaurora.org 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@us-west-2.amazonses.com/
>
>
> +- 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@...aro.org>
> > ---
> > 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;
> > }
Powered by blists - more mailing lists