On Wed, 18 Oct 2023 at 12:24, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
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;
--
With best wishes
Dmitry
