Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Thu, 2019-04-04 at 17:32 -0700, Souza, Jose wrote:
> On Thu, 2019-04-04 at 17:22 -0700, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> > > Even when driver is reloaded and hits this scenario the PSR mutex
> > > should be initialized, otherwise reading PSR debugfs status will
> > > execute mutex_lock() over a mutex that was not initialized.
> > >
> > > Cc: Dhinakaran Pandiyan
> > > Cc: Rodrigo Vivi
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/intel_psr.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index c80bb3003a7d..a84da931c3be 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > > if (val) {
> > > DRM_DEBUG_KMS("PSR interruption error set\n");
> > > dev_priv->psr.sink_not_reliable = true;
> >
> > Should we just sink_support = false and keep the return? IOW is there
> > any use
> > for sink_not_reliable?
>
> I guess it could cause confusion as user had PSR support before the
> module reload and after the load PSR debugfs will say that sink do not
> support PSR.
I don't think it is any more confusing than saying sink supports PSR and not
enabling it. Might as well make it clear that we are blaming the sink for not
enabling PSR.
>
> >
> > > - return;
> > > }
> > >
> > > /* Set link_standby x link_off defaults */
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Thu, 2019-04-04 at 17:22 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> > Even when driver is reloaded and hits this scenario the PSR mutex
> > should be initialized, otherwise reading PSR debugfs status will
> > execute mutex_lock() over a mutex that was not initialized.
> >
> > Cc: Dhinakaran Pandiyan
> > Cc: Rodrigo Vivi
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index c80bb3003a7d..a84da931c3be 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> > if (val) {
> > DRM_DEBUG_KMS("PSR interruption error set\n");
> > dev_priv->psr.sink_not_reliable = true;
> Should we just sink_support = false and keep the return? IOW is there
> any use
> for sink_not_reliable?
I guess it could cause confusion as user had PSR support before the
module reload and after the load PSR debugfs will say that sink do not
support PSR.
>
> > - return;
> > }
> >
> > /* Set link_standby x link_off defaults */
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> Even when driver is reloaded and hits this scenario the PSR mutex
> should be initialized, otherwise reading PSR debugfs status will
> execute mutex_lock() over a mutex that was not initialized.
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index c80bb3003a7d..a84da931c3be 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> if (val) {
> DRM_DEBUG_KMS("PSR interruption error set\n");
> dev_priv->psr.sink_not_reliable = true;
Should we just sink_support = false and keep the return? IOW is there any use
for sink_not_reliable?
> - return;
> }
>
> /* Set link_standby x link_off defaults */
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Thu, Apr 04, 2019 at 12:25:56PM -0700, Souza, Jose wrote:
> On Wed, 2019-04-03 at 17:27 -0700, Rodrigo Vivi wrote:
> > On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza
> > wrote:
> > > Even when driver is reloaded and hits this scenario the PSR mutex
> > > should be initialized, otherwise reading PSR debugfs status will
> > > execute mutex_lock() over a mutex that was not initialized.
> > >
> > > Cc: Dhinakaran Pandiyan
> > > Cc: Rodrigo Vivi
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/intel_psr.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index c80bb3003a7d..a84da931c3be 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > > if (val) {
> > > DRM_DEBUG_KMS("PSR interruption error set\n");
> > > dev_priv->psr.sink_not_reliable = true;
> > > - return;
> >
> > There are other returns above and if debugfs hits this case maybe it
> > is worth to move the mutex initialization up instead?
>
>
> We have those two returns in PSR debugfs, !HAS_PSR(dev_priv) and !psr-
> >sink_support and in this cases we don't have any PSR functionality so
> not worthy to initialize anything PSR related.
oh, indeed.
Reviewed-by: Rodrigo Vivi
>
> >
> > > }
> > >
> > > /* Set link_standby x link_off defaults */
> > > --
> > > 2.21.0
> > >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Wed, 2019-04-03 at 17:27 -0700, Rodrigo Vivi wrote:
> On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza
> wrote:
> > Even when driver is reloaded and hits this scenario the PSR mutex
> > should be initialized, otherwise reading PSR debugfs status will
> > execute mutex_lock() over a mutex that was not initialized.
> >
> > Cc: Dhinakaran Pandiyan
> > Cc: Rodrigo Vivi
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/intel_psr.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index c80bb3003a7d..a84da931c3be 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> > if (val) {
> > DRM_DEBUG_KMS("PSR interruption error set\n");
> > dev_priv->psr.sink_not_reliable = true;
> > - return;
>
> There are other returns above and if debugfs hits this case maybe it
> is worth to move the mutex initialization up instead?
We have those two returns in PSR debugfs, !HAS_PSR(dev_priv) and !psr-
>sink_support and in this cases we don't have any PSR functionality so
not worthy to initialize anything PSR related.
>
> > }
> >
> > /* Set link_standby x link_off defaults */
> > --
> > 2.21.0
> >
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable
On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza wrote:
> Even when driver is reloaded and hits this scenario the PSR mutex
> should be initialized, otherwise reading PSR debugfs status will
> execute mutex_lock() over a mutex that was not initialized.
>
> Cc: Dhinakaran Pandiyan
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index c80bb3003a7d..a84da931c3be 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
> if (val) {
> DRM_DEBUG_KMS("PSR interruption error set\n");
> dev_priv->psr.sink_not_reliable = true;
> - return;
There are other returns above and if debugfs hits this case maybe it
is worth to move the mutex initialization up instead?
> }
>
> /* Set link_standby x link_off defaults */
> --
> 2.21.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
