[PATCH] drm/i915: Fix DPLL warning when starting guest VM

[PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Zhao, Xinda]

The warning is occurred in guest VM when trying to get clock in encoder
initialization.

intel_modeset_init()
  ->intel_modeset_setup_hw_state()
    ->intel_ddi_get_config()
      ->intel_ddi_clock_get()
        ->skl_ddi_clock_get()
          ->intel_get_shared_dpll_id()
            ->WARN_ON(pll < dev_priv->shared_dplls||
                pll > &dev_priv->shared_dplls[dev_priv->num_shared_dpll])

In encoder initialization, shared DPLL is used for calculating clock for
DDI ports, but it is not set when crtc is not active.
In some cases, encoder is enabled while crtc is disabled, during encoder
initialization, the warning occurred.

Signed-off-by: Zhao, Xinda <xinda.zhao@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 28c25cb..ef35c12 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1489,6 +1489,16 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
+
+	/*
+	 * For DDI ports we always use a shared PLL.
+	 * But the shared PLL will not be set when crtc is not active.
+	 */
+	if (crtc->active == false) {
+		DRM_DEBUG_KMS("Trying to get clock, but pipe is not active.\n");
+		return;
+	}
 
 	if (INTEL_GEN(dev_priv) <= 8)
 		hsw_ddi_clock_get(encoder, pipe_config);
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for drm/i915: Fix DPLL warning when starting guest VM

[Patchwork]

== Series Details ==

Series: drm/i915: Fix DPLL warning when starting guest VM
URL   : https://patchwork.freedesktop.org/series/32820/
State : failure

== Summary ==

Series 32820v1 drm/i915: Fix DPLL warning when starting guest VM
https://patchwork.freedesktop.org/api/1.0/series/32820/revisions/1/mbox/

Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-bdw-5557u)
        Subgroup basic-reload-inject:
                pass       -> INCOMPLETE (fi-bdw-5557u)

fi-bdw-5557u     total:288  pass:266  dwarn:1   dfail:0   fail:0   skip:20 
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:450s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:526s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:265s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:492s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:494s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:472s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:599s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:575s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:484s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:425s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:426s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:422s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:454s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:492s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:576s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:582s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:541s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:590s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:644s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:513s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:453s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:558s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:418s

d0582552491e17f5e386747f82147c1f2d9158c9 drm-tip: 2017y-10m-27d-19h-16m-21s UTC integration manifest
2d90167d9c06 drm/i915: Fix DPLL warning when starting guest VM

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6257/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Ville Syrjälä]

On Mon, Oct 30, 2017 at 04:17:06PM +0800, Zhao, Xinda wrote:
> The warning is occurred in guest VM when trying to get clock in encoder
> initialization.
> 
> intel_modeset_init()
>   ->intel_modeset_setup_hw_state()
>     ->intel_ddi_get_config()
>       ->intel_ddi_clock_get()
>         ->skl_ddi_clock_get()
>           ->intel_get_shared_dpll_id()
>             ->WARN_ON(pll < dev_priv->shared_dplls||
>                 pll > &dev_priv->shared_dplls[dev_priv->num_shared_dpll])
> 
> In encoder initialization, shared DPLL is used for calculating clock for
> DDI ports, but it is not set when crtc is not active.
> In some cases, encoder is enabled while crtc is disabled, during encoder
> initialization, the warning occurred.
> 
> Signed-off-by: Zhao, Xinda <xinda.zhao@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 28c25cb..ef35c12 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1489,6 +1489,16 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>  			 struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> +
> +	/*
> +	 * For DDI ports we always use a shared PLL.
> +	 * But the shared PLL will not be set when crtc is not active.
> +	 */
> +	if (crtc->active == false) {

Seem to me that the correct fix would be to do the DPLL readout
regardless of the state of the pipe.

> +		DRM_DEBUG_KMS("Trying to get clock, but pipe is not active.\n");
> +		return;
> +	}
>  
>  	if (INTEL_GEN(dev_priv) <= 8)
>  		hsw_ddi_clock_get(encoder, pipe_config);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Ville Syrjälä]

On Mon, Oct 30, 2017 at 03:49:28PM +0200, Ville Syrjälä wrote:
> On Mon, Oct 30, 2017 at 04:17:06PM +0800, Zhao, Xinda wrote:
> > The warning is occurred in guest VM when trying to get clock in encoder
> > initialization.

What does guest VM mean here? gvt? If so, why do you claim to have
an enabled port without an enabled pipe?

> > 
> > intel_modeset_init()
> >   ->intel_modeset_setup_hw_state()
> >     ->intel_ddi_get_config()
> >       ->intel_ddi_clock_get()
> >         ->skl_ddi_clock_get()
> >           ->intel_get_shared_dpll_id()
> >             ->WARN_ON(pll < dev_priv->shared_dplls||
> >                 pll > &dev_priv->shared_dplls[dev_priv->num_shared_dpll])
> > 
> > In encoder initialization, shared DPLL is used for calculating clock for
> > DDI ports, but it is not set when crtc is not active.
> > In some cases, encoder is enabled while crtc is disabled, during encoder
> > initialization, the warning occurred.
> > 
> > Signed-off-by: Zhao, Xinda <xinda.zhao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 28c25cb..ef35c12 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1489,6 +1489,16 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
> >  			 struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> > +
> > +	/*
> > +	 * For DDI ports we always use a shared PLL.
> > +	 * But the shared PLL will not be set when crtc is not active.
> > +	 */
> > +	if (crtc->active == false) {
> 
> Seem to me that the correct fix would be to do the DPLL readout
> regardless of the state of the pipe.
> 
> > +		DRM_DEBUG_KMS("Trying to get clock, but pipe is not active.\n");
> > +		return;
> > +	}
> >  
> >  	if (INTEL_GEN(dev_priv) <= 8)
> >  		hsw_ddi_clock_get(encoder, pipe_config);
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

Re: [PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Zhao, Xinda]

Hi:

Thanks for your quickly response.
I have updated the comments in the following text

Thanks!

Best Wishes!
Xinda

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Monday, October 30, 2017 11:14 PM
> To: Zhao, Xinda <xinda.zhao@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zhao@freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix DPLL warning when starting
> guest VM
> 
> On Mon, Oct 30, 2017 at 03:49:28PM +0200, Ville Syrjälä wrote:
> > On Mon, Oct 30, 2017 at 04:17:06PM +0800, Zhao, Xinda wrote:
> > > The warning is occurred in guest VM when trying to get clock in
> > > encoder initialization.
> 
> What does guest VM mean here? gvt? If so, why do you claim to have an
> enabled port without an enabled pipe?

[xinda] 
Yes, gvt-g. 

We emulate a DP device on port B that is fixed to pipe A for each guest VM by setting following register, it is mandatory.
TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= (PORT_B << TRANS_DDI_PORT_SHIFT);

We don't emulate the status of pipe, whether it is enabled or not, it depends on the i915 setting in guest VM, it is optional.
The PIPECONF register will be trapped, but the behavior will not be emulated.

> 
> > >
> > > intel_modeset_init()
> > >   ->intel_modeset_setup_hw_state()
> > >     ->intel_ddi_get_config()
> > >       ->intel_ddi_clock_get()
> > >         ->skl_ddi_clock_get()
> > >           ->intel_get_shared_dpll_id()
> > >             ->WARN_ON(pll < dev_priv->shared_dplls||
> > >                 pll >
> > > &dev_priv->shared_dplls[dev_priv->num_shared_dpll])
> > >
> > > In encoder initialization, shared DPLL is used for calculating clock
> > > for DDI ports, but it is not set when crtc is not active.
> > > In some cases, encoder is enabled while crtc is disabled, during
> > > encoder initialization, the warning occurred.
> > >
> > > Signed-off-by: Zhao, Xinda <xinda.zhao@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 28c25cb..ef35c12 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -1489,6 +1489,16 @@ void intel_ddi_clock_get(struct intel_encoder
> *encoder,
> > >  			 struct intel_crtc_state *pipe_config)  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> > > +
> > > +	/*
> > > +	 * For DDI ports we always use a shared PLL.
> > > +	 * But the shared PLL will not be set when crtc is not active.
> > > +	 */
> > > +	if (crtc->active == false) {
> >
> > Seem to me that the correct fix would be to do the DPLL readout
> > regardless of the state of the pipe.

[xinda]
As many components in "struct intel_crtc_state pipe_config" are not reading out when crtc is inactive, like pixel_multiplier, fdi_lanes, fdi_m_n....,
should they all be read out?

I think the better choice is to add a judgement when the components are used, like my solution.


> >
> > > +		DRM_DEBUG_KMS("Trying to get clock, but pipe is not
> active.\n");
> > > +		return;
> > > +	}
> > >
> > >  	if (INTEL_GEN(dev_priv) <= 8)
> > >  		hsw_ddi_clock_get(encoder, pipe_config);
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1106 bytes --]

On 2017.10.31 03:09:04 +0000, Zhao, Xinda wrote:
> > 
> > On Mon, Oct 30, 2017 at 03:49:28PM +0200, Ville Syrjälä wrote:
> > > On Mon, Oct 30, 2017 at 04:17:06PM +0800, Zhao, Xinda wrote:
> > > > The warning is occurred in guest VM when trying to get clock in
> > > > encoder initialization.
> > 
> > What does guest VM mean here? gvt? If so, why do you claim to have an
> > enabled port without an enabled pipe?
> 
> [xinda] 
> Yes, gvt-g. 
> 
> We emulate a DP device on port B that is fixed to pipe A for each guest VM by setting following register, it is mandatory.
> TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= (PORT_B << TRANS_DDI_PORT_SHIFT);
> 
> We don't emulate the status of pipe, whether it is enabled or not, it depends on the i915 setting in guest VM, it is optional.
> The PIPECONF register will be trapped, but the behavior will not be emulated.
>

Looks that's wrong behavior which means full virtualized display brokenness
that still depends on some real hw status?

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm/i915: Fix DPLL warning when starting guest VM

[Zhao, Xinda]

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Tuesday, October 31, 2017 2:52 PM
> To: Zhao, Xinda <xinda.zhao@intel.com>
> Cc: ville.syrjala@linux.intel.com; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix DPLL warning when starting
> guest VM
> 
> On 2017.10.31 03:09:04 +0000, Zhao, Xinda wrote:
> > >
> > > On Mon, Oct 30, 2017 at 03:49:28PM +0200, Ville Syrjälä wrote:
> > > > On Mon, Oct 30, 2017 at 04:17:06PM +0800, Zhao, Xinda wrote:
> > > > > The warning is occurred in guest VM when trying to get clock in
> > > > > encoder initialization.
> > >
> > > What does guest VM mean here? gvt? If so, why do you claim to have
> > > an enabled port without an enabled pipe?
> >
> > [xinda]
> > Yes, gvt-g.
> >
> > We emulate a DP device on port B that is fixed to pipe A for each guest VM
> by setting following register, it is mandatory.
> > TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= (PORT_B <<
> TRANS_DDI_PORT_SHIFT);
> >
> > We don't emulate the status of pipe, whether it is enabled or not, it
> depends on the i915 setting in guest VM, it is optional.
> > The PIPECONF register will be trapped, but the behavior will not be
> emulated.
> >
> 
> Looks that's wrong behavior which means full virtualized display brokenness
> that still depends on some real hw status?

[xinda] 
Yes, consider the situation that PIPE A may be disabled in real hw, PIPE A should also be emulated in gvt, I will cook a new patch for this.
 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx