Re: [PATCH v1 2/3] drm/msm/dp: decoupling dp->id out of dp controller_id at scxxxx_dp_cfg table

[Dmitry Baryshkov]

On Sat, 25 Jun 2022 at 04:23, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
> On 6/24/2022 5:11 PM, Dmitry Baryshkov wrote:
> > On Sat, 25 Jun 2022 at 03:03, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
> >> On 6/24/2022 4:56 PM, Kuogee Hsieh wrote:
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> >> b/drivers/gpu/drm/msm/dp/dp_display.c
> >> index dcd80c8a794c..7816e82452ca 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> >> @@ -140,8 +140,8 @@ static const struct msm_dp_config sc7180_dp_cfg = {
> >>
> >>    static const struct msm_dp_config sc7280_dp_cfg = {
> >>           .descs = (const struct msm_dp_desc[]) {
> >> -               [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000,
> >> .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> >>                   [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea0000,
> >> .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
> >> +               [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000,
> >> .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
> >>           },
> >>           .num_descs = 2,
> >>    };
> >>
> >>
> >> The reason order is important is because  in this function below, even
> >> though it matches the address to find which one to use it loops through
> >> the array and so the value of *id will change depending on which one is
> >> located where.
> >>
> >> static const struct msm_dp_desc *dp_display_get_desc(struct
> >> platform_device *pdev,
> >>                                unsigned int *id)
> >> {
> >>       const struct msm_dp_config *cfg = of_device_get_match_data(&pdev->dev);
> >>       struct resource *res;
> >>       int i;
> >>
> >>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>       if (!res)
> >>           return NULL;
> >>
> >>       for (i = 0; i < cfg->num_descs; i++) {
> >>           if (cfg->descs[i].io_start == res->start) {
> >>               *id = i;
> >
> > The id is set to the index of the corresponding DP instance in the
> > descs array, which is MSM_DP_CONTROLLER_n. Correct up to now.
>
> Right, this is where I misunderstood his explanation.
>
> Even if we swap the order, but retain the index correctly it will still
> work today.
>
> Hes not sure of the root-cause of why turning on the primary display
> first fixes the issue.
>
> I think till we root-cause that, lets put this on hold.

Agreed. Let's find the root cause.


-- 
With best wishes
Dmitry