On Thu, Sep 19, 2019 at 07:23:30AM +0000, S, Srinivasan wrote:
> Would the following be appropriate fix?
>
> if (connector || connector->base.status == connector_status_connected) {
> ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> if (ret) {
> DRM_ERROR("failed to update payload %d\n", ret);
> }
> }
The whole connector->status check is racy. IMO don't do it.
>
> Regards,
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Manasi Navare
> Sent: Wednesday, September 18, 2019 11:55 PM
> To: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: S, Srinivasan <srinivasan.s@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable
>
> On Wed, Sep 18, 2019 at 09:11:36PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote:
> > > On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasan.s@xxxxxxxxx wrote:
> > > > From: Srinivasan S <srinivasan.s@xxxxxxxxx>
> > > >
> > > > This patch avoids DP MST payload error message in dmesg, as it is
> > > > trying to read the payload from the disconnected DP MST device.
> > > > After the unplug the connector status is disconnected and we
> > > > should not be looking for the payload and hence remove the error and throw the warning.
> > > >
> > > > This details can be found in:
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=111632
> > >
> > > Please add this link as Bugzilla:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=111632 after the Sign
> > > off statement
> > >
> > > >
> > > > Signed-off-by: Srinivasan S <srinivasan.s@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 ++++++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index eeeb3f933aa4..5b2278fdf675 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -215,7 +215,12 @@ static void intel_mst_disable_dp(struct
> > > > intel_encoder *encoder,
> > > >
> > > > ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> > > > if (ret) {
> > > > - DRM_ERROR("failed to update payload %d\n", ret);
> > > > + if (!connector ||
> > > > + connector->base.status != connector_status_connected) {
> > > > + DRM_WARN("DP MST disconnect\n");
> > >
> > > May be adding this check before calling drm_dp_update_payload_part1() is a better idea?
> > > If the connector is disconnected, why update payload?
> > >
> > > Jani, Ville, thoughts?
> >
> > Or just convert it to a debug?
>
> Sure that will work, but do we really want to update the payload if the connector status is disconnected.
> So shouldnt checking that before calling the function be a better fix?
>
> Manasi
>
> >
> > >
> > > Regards
> > > Manasi
> > >
> > > > + } else {
> > > > + DRM_ERROR("failed to update payload %d\n", ret);
> > > > + }
> > > > }
> > > > if (old_crtc_state->has_audio)
> > > > intel_audio_codec_disable(encoder,
> > > > --
> > > > 2.7.4
> > > >
> >
> > --
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
