Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-10-01 Thread S, Srinivasan 
Thanks a lot Manasi, Ville, Mika, Jani, Lakshmi, for all your time in reviewing 
this patch.

Best Regards,

> -Original Message-
> From: dri-devel  On Behalf Of Ville
> Syrjälä
> Sent: Tuesday, October 1, 2019 5:31 PM
> To: S, Srinivasan 
> Cc: Navare, Manasi D ; intel-
> g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Vudum,
> Lakshminarayana 
> Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC
> cable
> 
> On Wed, Sep 25, 2019 at 06:05:42AM +0530, srinivasa...@intel.com wrote:
> > From: Srinivasan S 
> >
> > This patch avoids DP MST payload error message in dmesg, as it is trying
> > to update the payload to the disconnected DP MST device. After DP MST
> > device is disconnected we should not be updating the payload and
> > hence remove the error.
> >
> > v2: Removed the connector status check and converted from error to debug.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111632
> > Signed-off-by: Srinivasan S 
> 
> Pushed to dinq. Thanks for the patch.
> 
> PS. Next time please use --in-reply-to when sending an updated patch
> so that it's easier to keep track of the discussion.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> >  1 file changed, 1 insertion(+), 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..497a6ae0d2c0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -215,7 +215,7 @@ static void intel_mst_disable_dp(struct intel_encoder
> *encoder,
> >
> > ret = drm_dp_update_payload_part1(_dp->mst_mgr);
> > if (ret) {
> > -   DRM_ERROR("failed to update payload %d\n", ret);
> > +   DRM_DEBUG_KMS("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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-10-01 Thread Ville Syrjälä 
On Wed, Sep 25, 2019 at 06:05:42AM +0530, srinivasa...@intel.com wrote:
> From: Srinivasan S 
> 
> This patch avoids DP MST payload error message in dmesg, as it is trying
> to update the payload to the disconnected DP MST device. After DP MST
> device is disconnected we should not be updating the payload and
> hence remove the error.
> 
> v2: Removed the connector status check and converted from error to debug.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111632
> Signed-off-by: Srinivasan S 

Pushed to dinq. Thanks for the patch.

PS. Next time please use --in-reply-to when sending an updated patch
so that it's easier to keep track of the discussion.

> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
>  1 file changed, 1 insertion(+), 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..497a6ae0d2c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -215,7 +215,7 @@ static void intel_mst_disable_dp(struct intel_encoder 
> *encoder,
>  
>   ret = drm_dp_update_payload_part1(_dp->mst_mgr);
>   if (ret) {
> - DRM_ERROR("failed to update payload %d\n", ret);
> + DRM_DEBUG_KMS("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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-30 Thread S, Srinivasan 
Could anyone please review the patch below & let me know if any other ideas 
please?

https://patchwork.freedesktop.org/patch/332806/?series=66837=3

Thanks,

> -Original Message-
> From: S, Srinivasan
> Sent: Wednesday, September 25, 2019 8:33 PM
> To: 'Ville Syrjälä' 
> Cc: Navare, Manasi D ; 'intel-
> g...@lists.freedesktop.org' ; 'dri-
> de...@lists.freedesktop.org' 
> Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC
> cable
> 
> Hi Ville,
> 
> I have revised the patch from DRM_ERROR to DRM_DEBUG, could you please
> review?
> 
> https://patchwork.freedesktop.org/patch/332806/?series=66837=3
> 
> Thanks,
> 
> -Original Message-
> From: S, Srinivasan
> Sent: Thursday, September 19, 2019 7:22 PM
> To: 'Ville Syrjälä' 
> Cc: Navare, Manasi D ; 'intel-
> g...@lists.freedesktop.org' ; 'dri-
> de...@lists.freedesktop.org' 
> Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC
> cable
> 
> Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update
> payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n",
> ret);, without any connector status check, would that be fine?
> 
> Regards,
> -Original Message-
> From: Ville Syrjälä 
> Sent: Thursday, September 19, 2019 5:34 PM
> To: S, Srinivasan 
> Cc: Navare, Manasi D ; intel-
> g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC
> cable
> 
> On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote:
> > Would the following be appropriate fix?
> >
> > if (connector || connector->base.status == 
> > connector_status_connected)
> {
> > ret = drm_dp_update_payload_part1(_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  On Behalf Of
> > Manasi Navare
> > Sent: Wednesday, September 18, 2019 11:55 PM
> > To: Ville Syrjälä 
> > Cc: S, Srinivasan ;
> > intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> > 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, srinivasa...@intel.com
> wrote:
> > > > > From: Srinivasan S 
> > > > >
> > > > > 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 
> > > > > ---
> > > > >  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(_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

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-25 Thread S, Srinivasan 
Hi Ville,

I have revised the patch from DRM_ERROR to DRM_DEBUG, could you please review?

https://patchwork.freedesktop.org/patch/332806/?series=66837=3

Thanks,

-Original Message-
From: S, Srinivasan 
Sent: Thursday, September 19, 2019 7:22 PM
To: 'Ville Syrjälä' 
Cc: Navare, Manasi D ; 
'intel-gfx@lists.freedesktop.org' ; 
'dri-de...@lists.freedesktop.org' 
Subject: RE: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update 
payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n", 
ret);, without any connector status check, would that be fine?

Regards,
-Original Message-
From: Ville Syrjälä 
Sent: Thursday, September 19, 2019 5:34 PM
To: S, Srinivasan 
Cc: Navare, Manasi D ; 
intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote:
> Would the following be appropriate fix?
> 
> if (connector || connector->base.status == 
> connector_status_connected) {
> ret = drm_dp_update_payload_part1(_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  On Behalf Of 
> Manasi Navare
> Sent: Wednesday, September 18, 2019 11:55 PM
> To: Ville Syrjälä 
> Cc: S, Srinivasan ; 
> intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> 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, srinivasa...@intel.com wrote:
> > > > From: Srinivasan S 
> > > > 
> > > > 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 
> > > > ---
> > > >  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(_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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-24 Thread srinivasan . s 
From: Srinivasan S 

This patch avoids DP MST payload error message in dmesg, as it is trying
to update the payload to the disconnected DP MST device. After DP MST
device is disconnected we should not be updating the payload and
hence remove the error.

v2: Removed the connector status check and converted from error to debug.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111632
Signed-off-by: Srinivasan S 
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
 1 file changed, 1 insertion(+), 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..497a6ae0d2c0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -215,7 +215,7 @@ static void intel_mst_disable_dp(struct intel_encoder 
*encoder,
 
ret = drm_dp_update_payload_part1(_dp->mst_mgr);
if (ret) {
-   DRM_ERROR("failed to update payload %d\n", ret);
+   DRM_DEBUG_KMS("failed to update payload %d\n", ret);
}
if (old_crtc_state->has_audio)
intel_audio_codec_disable(encoder,
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-19 Thread S, Srinivasan 
Then it's better that, could we change it to DRM_DEBUG_KMS("failed to update 
payload %d\n", ret); instead of DRM_ERROR("failed to update payload %d\n", 
ret);, without any connector status check, would that be fine?

Regards,
-Original Message-
From: Ville Syrjälä  
Sent: Thursday, September 19, 2019 5:34 PM
To: S, Srinivasan 
Cc: Navare, Manasi D ; 
intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote:
> Would the following be appropriate fix?
> 
> if (connector || connector->base.status == 
> connector_status_connected) {
> ret = drm_dp_update_payload_part1(_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  On Behalf Of 
> Manasi Navare
> Sent: Wednesday, September 18, 2019 11:55 PM
> To: Ville Syrjälä 
> Cc: S, Srinivasan ; 
> intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> 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, srinivasa...@intel.com wrote:
> > > > From: Srinivasan S 
> > > > 
> > > > 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 
> > > > ---
> > > >  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(_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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-19 Thread Jani Nikula 
On Wed, 18 Sep 2019, Manasi Navare  wrote:
> On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote:
>> From: Srinivasan S 
>> 
>> 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

ITYM *before* Signed-off-by. But yeah, use the Bugzilla: tag, and please
use git log to see plenty of examples.

BR,
Jani.

>
>> 
>> Signed-off-by: Srinivasan S 
>> ---
>>  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(_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?
>
> 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
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-19 Thread Ville Syrjälä 
On Thu, Sep 19, 2019 at 07:23:30AM +, S, Srinivasan wrote:
> Would the following be appropriate fix?
> 
> if (connector || connector->base.status == 
> connector_status_connected) {
> ret = drm_dp_update_payload_part1(_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  On Behalf Of Manasi 
> Navare
> Sent: Wednesday, September 18, 2019 11:55 PM
> To: Ville Syrjälä 
> Cc: S, Srinivasan ; intel-gfx@lists.freedesktop.org; 
> dri-de...@lists.freedesktop.org
> 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, srinivasa...@intel.com wrote:
> > > > From: Srinivasan S 
> > > > 
> > > > 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 
> > > > ---
> > > >  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(_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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-19 Thread S, Srinivasan 
Would the following be appropriate fix?

if (connector || connector->base.status == connector_status_connected) {
ret = drm_dp_update_payload_part1(_dp->mst_mgr);
if (ret) {
DRM_ERROR("failed to update payload %d\n", ret);
}
}

Regards,
-Original Message-
From: dri-devel  On Behalf Of Manasi 
Navare
Sent: Wednesday, September 18, 2019 11:55 PM
To: Ville Syrjälä 
Cc: S, Srinivasan ; intel-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org
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, srinivasa...@intel.com wrote:
> > > From: Srinivasan S 
> > > 
> > > 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 
> > > ---
> > >  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(_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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-18 Thread Manasi Navare 
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, srinivasa...@intel.com wrote:
> > > From: Srinivasan S 
> > > 
> > > 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 
> > > ---
> > >  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(_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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-18 Thread Ville Syrjälä 
On Wed, Sep 18, 2019 at 10:50:39AM -0700, Manasi Navare wrote:
> On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote:
> > From: Srinivasan S 
> > 
> > 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 
> > ---
> >  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(_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?

> 
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-18 Thread Manasi Navare 
On Wed, Sep 18, 2019 at 07:09:43AM +0530, srinivasa...@intel.com wrote:
> From: Srinivasan S 
> 
> 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 
> ---
>  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(_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?

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
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/dp: Fix DP MST error after unplugging TypeC cable

2019-09-17 Thread srinivasan . s 
From: Srinivasan S 

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

Signed-off-by: Srinivasan S 
---
 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(_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");
+   } else {
+   DRM_ERROR("failed to update payload %d\n", ret);
+   }
}
if (old_crtc_state->has_audio)
intel_audio_codec_disable(encoder,
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx