| Message ID | 20231018074627.55637-1-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dp: attach the DP subconnector property | expand |
On Wed, 18 Oct 2023 at 10:46, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: > support setting the DP subconnector type") I had the patch [1] in my > tree. I haven't noticed that it was a dependency for the commit in > question. Mea culpa. [1] https://patchwork.freedesktop.org/patch/555530/ > Since the patch has not landed yet (and even was not reviewed) > and since one of the bridges erroneously uses USB connector type instead > of DP, attach the property directly from the MSM DP driver. > > This fixes the following oops on DP HPD event: > > drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) > dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) > dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) > hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) > kthread (kernel/kthread.c:388) > ret_from_fork (arch/arm64/kernel/entry.S:858)
On 10/18/2023 12:46 AM, Dmitry Baryshkov wrote: > While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: > support setting the DP subconnector type") I had the patch [1] in my > tree. I haven't noticed that it was a dependency for the commit in > question. Mea culpa. > I agree with you that, we should be setting this in the framework is better. Will review that one on the other patch. But yes, we need to fix this regression first. > Since the patch has not landed yet (and even was not reviewed) > and since one of the bridges erroneously uses USB connector type instead > of DP, attach the property directly from the MSM DP driver. > > This fixes the following oops on DP HPD event: > > drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) > dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) > dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) > hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) > kthread (kernel/kthread.c:388) > ret_from_fork (arch/arm64/kernel/entry.S:858) > > Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> One question, while reviewing the code, I see you have two calls to drm_dp_set_subconnector_property() for the connect and disconnect case. Why cant we have just one call in dp_display_send_hpd_notification() for both cases? > drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c > index 40e7344180e3..e3bdd7dd4cdc 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > @@ -345,6 +345,9 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr > if (IS_ERR(connector)) > return connector; > > + if (!dp_display->is_edp) > + drm_connector_attach_dp_subconnector_property(connector); > + > drm_connector_attach_encoder(connector, encoder); > > return connector;
On Wed, 18 Oct 2023 at 12:24, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 10/18/2023 12:46 AM, Dmitry Baryshkov wrote: > > While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: > > support setting the DP subconnector type") I had the patch [1] in my > > tree. I haven't noticed that it was a dependency for the commit in > > question. Mea culpa. > > > > I agree with you that, we should be setting this in the framework is better. > > Will review that one on the other patch. > > But yes, we need to fix this regression first. > > > Since the patch has not landed yet (and even was not reviewed) > > and since one of the bridges erroneously uses USB connector type instead > > of DP, attach the property directly from the MSM DP driver. > > > > This fixes the following oops on DP HPD event: > > > > drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) > > dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) > > dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) > > hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) > > kthread (kernel/kthread.c:388) > > ret_from_fork (arch/arm64/kernel/entry.S:858) > > > > Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Just to note, I'm going to send v2 in the next few days, fixing this code to also work in eDP. > > One question, while reviewing the code, I see you have two calls to > drm_dp_set_subconnector_property() for the connect and disconnect case. > > Why cant we have just one call in dp_display_send_hpd_notification() for > both cases? Hmm, I'll have to check. > > > > drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c > > index 40e7344180e3..e3bdd7dd4cdc 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > > @@ -345,6 +345,9 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr > > if (IS_ERR(connector)) > > return connector; > > > > + if (!dp_display->is_edp) > > + drm_connector_attach_dp_subconnector_property(connector); > > + > > drm_connector_attach_encoder(connector, encoder); > > > > return connector;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index 40e7344180e3..e3bdd7dd4cdc 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -345,6 +345,9 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr if (IS_ERR(connector)) return connector; + if (!dp_display->is_edp) + drm_connector_attach_dp_subconnector_property(connector); + drm_connector_attach_encoder(connector, encoder); return connector;
While developing and testing the commit bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") I had the patch [1] in my tree. I haven't noticed that it was a dependency for the commit in question. Mea culpa. Since the patch has not landed yet (and even was not reviewed) and since one of the bridges erroneously uses USB connector type instead of DP, attach the property directly from the MSM DP driver. This fixes the following oops on DP HPD event: drm_object_property_set_value (drivers/gpu/drm/drm_mode_object.c:288) dp_display_process_hpd_high (drivers/gpu/drm/msm/dp/dp_display.c:402) dp_hpd_plug_handle.isra.0 (drivers/gpu/drm/msm/dp/dp_display.c:604) hpd_event_thread (drivers/gpu/drm/msm/dp/dp_display.c:1110) kthread (kernel/kthread.c:388) ret_from_fork (arch/arm64/kernel/entry.S:858) Fixes: bfcc3d8f94f4 ("drm/msm/dp: support setting the DP subconnector type") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++ 1 file changed, 3 insertions(+)