| Message ID | 1696436821-14261-1-git-send-email-quic_khsieh@quicinc.com |
|---|---|
| Headers | show |
| Series | incorporate pm runtime framework and eDP clean up | expand |
On 04/10/2023 19:26, Kuogee Hsieh wrote: > Currently the dp_display_request_irq() is executed at msm_dp_modeset_init() > which ties irq registering to the DPU device's life cycle, while depending on > resources that are released as the DP device is torn down. Move register DP > driver irq handler to dp_display_probe() to have dp_display_irq_handler() > IRQ tied with DP device. In addition, use platform_get_irq() to retrieve irq > number from platform device directly. > > Changes in v5: > -- reworded commit text as review comments at change #4 > -- tear down component if failed at dp_display_request_irq() > > Changes in v4: > -- delete dp->irq check at dp_display_request_irq() > > Changes in v3: > -- move calling dp_display_irq_handler() to probe > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 32 +++++++++++++------------------- > drivers/gpu/drm/msm/dp/dp_display.h | 1 - > 2 files changed, 13 insertions(+), 20 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 04/10/2023 19:26, Kuogee Hsieh wrote: > The is_connected flag is set to true after DP mainlink successfully > finishes link training to enter into ST_MAINLINK_READY state rather > than being set after the DP dongle is connected. Rename the > is_connected flag with link_ready flag to match the state of DP > driver's state machine. > > Changes in v5: > -- reworded commit text according to review comments from change #4 > > Changes in v4: > -- reworded commit text > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 19 +++++++++---------- > drivers/gpu/drm/msm/dp/dp_display.h | 2 +- > drivers/gpu/drm/msm/dp/dp_drm.c | 14 +++++++------- > 3 files changed, 17 insertions(+), 18 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 04/10/2023 19:26, Kuogee Hsieh wrote: > Original both parser->parse() and dp_power_client_init() are done at > dp_display_bind() since eDP population is done at binding time. > In the preparation of having eDP population done at probe() time, > move both function from dp_display_bind() to dp_display_probe(). > > Changes in v4: > -- explain why parser->parse() and dp_power_client_init() are moved to probe time > -- tear down sub modules if failed > > Changes in v4: > -- split this patch out of "incorporate pm_runtime framework into DP driver" patch > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 32663ea..e4942fc 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -276,11 +276,6 @@ static int dp_display_bind(struct device *dev, struct device *master, > dp->dp_display.drm_dev = drm; > priv->dp[dp->id] = &dp->dp_display; > > - rc = dp->parser->parse(dp->parser); > - if (rc) { > - DRM_ERROR("device tree parsing failed\n"); > - goto end; > - } > > > dp->drm_dev = drm; > @@ -291,11 +286,6 @@ static int dp_display_bind(struct device *dev, struct device *master, > goto end; > } > > - rc = dp_power_client_init(dp->power); As we have moved dp_power_client_init() from bind() to probe(), we should also move dp_power_client_deinit() to remove(). Otherwise after several bind/unbind attempts the pm enablement count can become negative. > - if (rc) { > - DRM_ERROR("Power client create failed\n"); > - goto end; > - } > > rc = dp_register_audio_driver(dev, dp->audio); > if (rc) { > @@ -1250,6 +1240,18 @@ static int dp_display_probe(struct platform_device *pdev) > return -EPROBE_DEFER; > } > > + rc = dp->parser->parse(dp->parser); > + if (rc) { > + DRM_ERROR("device tree parsing failed\n"); > + goto err; > + } > + > + rc = dp_power_client_init(dp->power); > + if (rc) { > + DRM_ERROR("Power client create failed\n"); > + goto err; > + } > + > /* setup event q */ > mutex_init(&dp->event_mutex); > init_waitqueue_head(&dp->event_q);
On 04/10/2023 19:27, Kuogee Hsieh wrote: > Currently eDP population is done at msm_dp_modeset_init() which happen > at binding time. Move eDP population to be done at display probe time > so that probe deferral cases can be handled effectively. > wait_for_hpd_asserted callback is added during drm_dp_aux_init() > to ensure eDP's HPD is up before proceeding eDP population. > > Changes in v5: > -- inline dp_display_auxbus_population() and delete it > > Changes in v4: > -- delete duplicate initialize code to dp_aux before drm_dp_aux_register() > -- delete of_get_child_by_name(dev->of_node, "aux-bus") and inline the function > -- not initialize rc = 0 > > Changes in v3: > -- add done_probing callback into devm_of_dp_aux_populate_bus() > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_aux.c | 34 ++++++++++++++++----- > drivers/gpu/drm/msm/dp/dp_display.c | 59 +++++++++++++++---------------------- > 2 files changed, 51 insertions(+), 42 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c > index 10b6eeb..03f4951 100644 > --- a/drivers/gpu/drm/msm/dp/dp_aux.c > +++ b/drivers/gpu/drm/msm/dp/dp_aux.c > @@ -479,7 +479,6 @@ void dp_aux_deinit(struct drm_dp_aux *dp_aux) > > int dp_aux_register(struct drm_dp_aux *dp_aux) > { > - struct dp_aux_private *aux; > int ret; > > if (!dp_aux) { > @@ -487,12 +486,7 @@ int dp_aux_register(struct drm_dp_aux *dp_aux) > return -EINVAL; > } > > - aux = container_of(dp_aux, struct dp_aux_private, dp_aux); > - > - aux->dp_aux.name = "dpu_dp_aux"; > - aux->dp_aux.dev = aux->dev; > - aux->dp_aux.transfer = dp_aux_transfer; > - ret = drm_dp_aux_register(&aux->dp_aux); > + ret = drm_dp_aux_register(dp_aux); > if (ret) { > DRM_ERROR("%s: failed to register drm aux: %d\n", __func__, > ret); > @@ -507,6 +501,21 @@ void dp_aux_unregister(struct drm_dp_aux *dp_aux) > drm_dp_aux_unregister(dp_aux); > } > > +static int dp_wait_hpd_asserted(struct drm_dp_aux *dp_aux, > + unsigned long wait_us) > +{ > + int ret; > + struct dp_aux_private *aux; > + > + aux = container_of(dp_aux, struct dp_aux_private, dp_aux); > + > + pm_runtime_get_sync(aux->dev); > + ret = dp_catalog_aux_wait_for_hpd_connect_state(aux->catalog); > + pm_runtime_put_sync(aux->dev); > + > + return ret; > +} > + > struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog, > bool is_edp) > { > @@ -530,6 +539,17 @@ struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog, > aux->catalog = catalog; > aux->retry_cnt = 0; > > + /* > + * Use the drm_dp_aux_init() to use the aux adapter > + * before registering AUX with the DRM device so that > + * msm eDP panel can be detected by generic_dep_panel_probe(). > + */ > + aux->dp_aux.name = "dpu_dp_aux"; > + aux->dp_aux.dev = dev; > + aux->dp_aux.transfer = dp_aux_transfer; > + aux->dp_aux.wait_hpd_asserted = dp_wait_hpd_asserted; > + drm_dp_aux_init(&aux->dp_aux); > + > return &aux->dp_aux; > } > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 79e56d9..df15145 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1207,6 +1207,17 @@ static const struct msm_dp_desc *dp_display_get_desc(struct platform_device *pde > return NULL; > } > > +static int dp_auxbus_done_probe(struct drm_dp_aux *aux) > +{ > + int rc; > + > + rc = component_add(aux->dev, &dp_display_comp_ops); > + if (rc) > + DRM_ERROR("eDP component add failed, rc=%d\n", rc); > + > + return rc; > +} > + > static int dp_display_probe(struct platform_device *pdev) > { > int rc = 0; > @@ -1272,10 +1283,18 @@ static int dp_display_probe(struct platform_device *pdev) > if (rc) > goto err; > > - rc = component_add(&pdev->dev, &dp_display_comp_ops); > - if (rc) { > - DRM_ERROR("component add failed, rc=%d\n", rc); > - goto err; > + if (dp->dp_display.is_edp) { > + rc = devm_of_dp_aux_populate_bus(dp->aux, dp_auxbus_done_probe); > + if (rc) { > + DRM_ERROR("eDP auxbus population failed, rc=%d\n", rc); > + goto err; > + } > + } else { > + rc = component_add(&pdev->dev, &dp_display_comp_ops); > + if (rc) { > + DRM_ERROR("component add failed, rc=%d\n", rc); > + goto err; > + } > } > > return rc; > @@ -1291,7 +1310,6 @@ static int dp_display_remove(struct platform_device *pdev) > > component_del(&pdev->dev, &dp_display_comp_ops); > dp_display_deinit_sub_modules(dp); > - Nit: irrelevant to this patch. > platform_set_drvdata(pdev, NULL); > > return 0; > @@ -1388,29 +1406,8 @@ static int dp_display_get_next_bridge(struct msm_dp *dp) > { > int rc; > struct dp_display_private *dp_priv; > - struct device_node *aux_bus; > - struct device *dev; > > dp_priv = container_of(dp, struct dp_display_private, dp_display); > - dev = &dp_priv->pdev->dev; > - aux_bus = of_get_child_by_name(dev->of_node, "aux-bus"); > - > - if (aux_bus && dp->is_edp) { > - /* > - * The code below assumes that the panel will finish probing > - * by the time devm_of_dp_aux_populate_ep_devices() returns. > - * This isn't a great assumption since it will fail if the > - * panel driver is probed asynchronously but is the best we > - * can do without a bigger driver reorganization. > - */ > - rc = of_dp_aux_populate_bus(dp_priv->aux, NULL); > - of_node_put(aux_bus); > - if (rc) > - goto error; > - } else if (dp->is_edp) { > - DRM_ERROR("eDP aux_bus not found\n"); > - return -ENODEV; > - } > > /* > * External bridges are mandatory for eDP interfaces: one has to > @@ -1423,17 +1420,9 @@ static int dp_display_get_next_bridge(struct msm_dp *dp) > if (!dp->is_edp && rc == -ENODEV) > return 0; > > - if (!rc) { > + if (!rc) > dp->next_bridge = dp_priv->parser->next_bridge; > - return 0; > - } > > -error: > - if (dp->is_edp) { > - of_dp_aux_depopulate_bus(dp_priv->aux); > - dp_display_host_phy_exit(dp_priv); > - dp_display_host_deinit(dp_priv); > - } > return rc; > } >