drm/i915/gvt: Disable primary/sprite/cursor plane at vdisplay initialization

Submitted by Xiong Zhang on March 24, 2018, 1:45 a.m.

Details

Message ID 1521855959-4367-1-git-send-email-xiong.y.zhang@intel.com
State New
Headers show
Series "drm/i915/gvt: Disable primary/sprite/cursor plane at vdisplay initialization" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Xiong Zhang March 24, 2018, 1:45 a.m.
Many error exist in host dmesg during guest boot up with loca display
enabled.
gvt: vgpu 1: invalid range gmadr 0x0 size 0x0
gvt: vgpu 1: invalid gma address: 0

This error happens when qemu get dmabuf info where the vdisplay plane 
is enabled but its base address is a invalid 0, this status may be true 
before guest enable its plane. At this moment, its state is copied from 
host where the plane may be enabled.

This patch disable primary/sprite/cursor plane at vdisplay
initialization to remove the above error.

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/display.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index dd96ffc..6d8180e 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -169,6 +169,8 @@  static u8 dpcd_fix_data[DPCD_HEADER_SIZE] = {
 static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int pipe;
+
 	vgpu_vreg_t(vgpu, SDEISR) &= ~(SDE_PORTB_HOTPLUG_CPT |
 			SDE_PORTC_HOTPLUG_CPT |
 			SDE_PORTD_HOTPLUG_CPT);
@@ -267,6 +269,14 @@  static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
 	if (IS_BROADWELL(dev_priv))
 		vgpu_vreg_t(vgpu, PCH_ADPA) &= ~ADPA_CRT_HOTPLUG_MONITOR_MASK;
 
+	/* Disable Primary/Sprite/Cursor plane */
+	for_each_pipe(dev_priv, pipe) {
+		vgpu_vreg_t(vgpu, DSPCNTR(pipe)) &= ~DISPLAY_PLANE_ENABLE;
+		vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE;
+		vgpu_vreg_t(vgpu, CURCNTR(pipe)) &= ~CURSOR_MODE;
+		vgpu_vreg_t(vgpu, CURCNTR(pipe)) |= CURSOR_MODE_DISABLE;
+	}
+
 	vgpu_vreg_t(vgpu, PIPECONF(PIPE_A)) |= PIPECONF_ENABLE;
 }
 

Comments

On 2018.03.24 09:45:59 +0800, Xiong Zhang wrote:
> Many error exist in host dmesg during guest boot up with loca display
> enabled.
> gvt: vgpu 1: invalid range gmadr 0x0 size 0x0
> gvt: vgpu 1: invalid gma address: 0
> 
> This error happens when qemu get dmabuf info where the vdisplay plane 
> is enabled but its base address is a invalid 0, this status may be true 
> before guest enable its plane. At this moment, its state is copied from 
> host where the plane may be enabled.

hmm, even it's enabled, dmabuf code should have checked address in that case, right?
So this is actually to remove those error message? maybe just turn them into debug level instead.

> 
> This patch disable primary/sprite/cursor plane at vdisplay
> initialization to remove the above error.
>

I think the display driver shouldn't have any assumption on plane enable,
so this is not a must.

btw, 'vdisplay' is not a formal word for us, better write complete virtual display.

> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/display.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index dd96ffc..6d8180e 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -169,6 +169,8 @@ static u8 dpcd_fix_data[DPCD_HEADER_SIZE] = {
>  static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>  {
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> +	int pipe;
> +
>  	vgpu_vreg_t(vgpu, SDEISR) &= ~(SDE_PORTB_HOTPLUG_CPT |
>  			SDE_PORTC_HOTPLUG_CPT |
>  			SDE_PORTD_HOTPLUG_CPT);
> @@ -267,6 +269,14 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>  	if (IS_BROADWELL(dev_priv))
>  		vgpu_vreg_t(vgpu, PCH_ADPA) &= ~ADPA_CRT_HOTPLUG_MONITOR_MASK;
>  
> +	/* Disable Primary/Sprite/Cursor plane */
> +	for_each_pipe(dev_priv, pipe) {
> +		vgpu_vreg_t(vgpu, DSPCNTR(pipe)) &= ~DISPLAY_PLANE_ENABLE;
> +		vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE;
> +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) &= ~CURSOR_MODE;
> +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) |= CURSOR_MODE_DISABLE;
> +	}
> +
>  	vgpu_vreg_t(vgpu, PIPECONF(PIPE_A)) |= PIPECONF_ENABLE;
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> On 2018.03.24 09:45:59 +0800, Xiong Zhang wrote:
> > Many error exist in host dmesg during guest boot up with loca display
> > enabled.
> > gvt: vgpu 1: invalid range gmadr 0x0 size 0x0
> > gvt: vgpu 1: invalid gma address: 0
> >
> > This error happens when qemu get dmabuf info where the vdisplay plane
> > is enabled but its base address is a invalid 0, this status may be
> > true before guest enable its plane. At this moment, its state is
> > copied from host where the plane may be enabled.
> 
> hmm, even it's enabled, dmabuf code should have checked address in that
> case, right?
[Zhang, Xiong Y] Yes, dmabuf code has checked the address, and the error message happens in this checking code.

> So this is actually to remove those error message? maybe just turn them into
> debug level instead.
[Zhang, Xiong Y] yes, the purpose is to remove this heavy error message. This should be an error level, if guest actually set a bad base address, error message should be pop up.

Qemu are probing dma buf info at each qemu display refresh. Before guest driver enable the plane, its initial state should be disabled, then intel_vgpu_decode_primary/cursor/sprite_plane could return early, and avoid to check the plane base address. Finally error message will disappear before guest driver enable the plane.
> 
> >
> > This patch disable primary/sprite/cursor plane at vdisplay
> > initialization to remove the above error.
> >
> 
> I think the display driver shouldn't have any assumption on plane enable, so
> this is not a must.
> 
> btw, 'vdisplay' is not a formal word for us, better write complete virtual
> display.
[Zhang, Xiong Y] ok, I will use full name "virtual display"
> 
> > Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/display.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/display.c
> > b/drivers/gpu/drm/i915/gvt/display.c
> > index dd96ffc..6d8180e 100644
> > --- a/drivers/gpu/drm/i915/gvt/display.c
> > +++ b/drivers/gpu/drm/i915/gvt/display.c
> > @@ -169,6 +169,8 @@ static u8 dpcd_fix_data[DPCD_HEADER_SIZE] = {
> > static void emulate_monitor_status_change(struct intel_vgpu *vgpu)  {
> >  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> > +	int pipe;
> > +
> >  	vgpu_vreg_t(vgpu, SDEISR) &= ~(SDE_PORTB_HOTPLUG_CPT |
> >  			SDE_PORTC_HOTPLUG_CPT |
> >  			SDE_PORTD_HOTPLUG_CPT);
> > @@ -267,6 +269,14 @@ static void
> emulate_monitor_status_change(struct intel_vgpu *vgpu)
> >  	if (IS_BROADWELL(dev_priv))
> >  		vgpu_vreg_t(vgpu, PCH_ADPA) &=
> ~ADPA_CRT_HOTPLUG_MONITOR_MASK;
> >
> > +	/* Disable Primary/Sprite/Cursor plane */
> > +	for_each_pipe(dev_priv, pipe) {
> > +		vgpu_vreg_t(vgpu, DSPCNTR(pipe)) &= ~DISPLAY_PLANE_ENABLE;
> > +		vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE;
> > +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) &= ~CURSOR_MODE;
> > +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) |= CURSOR_MODE_DISABLE;
> > +	}
> > +
> >  	vgpu_vreg_t(vgpu, PIPECONF(PIPE_A)) |= PIPECONF_ENABLE;  }
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
On 2018.03.26 02:53:11 +0000, Zhang, Xiong Y wrote:
> > On 2018.03.24 09:45:59 +0800, Xiong Zhang wrote:
> > > Many error exist in host dmesg during guest boot up with loca display
> > > enabled.
> > > gvt: vgpu 1: invalid range gmadr 0x0 size 0x0
> > > gvt: vgpu 1: invalid gma address: 0
> > >
> > > This error happens when qemu get dmabuf info where the vdisplay plane
> > > is enabled but its base address is a invalid 0, this status may be
> > > true before guest enable its plane. At this moment, its state is
> > > copied from host where the plane may be enabled.
> > 
> > hmm, even it's enabled, dmabuf code should have checked address in that
> > case, right?
> [Zhang, Xiong Y] Yes, dmabuf code has checked the address, and the error message happens in this checking code.
> 
> > So this is actually to remove those error message? maybe just turn them into
> > debug level instead.
> [Zhang, Xiong Y] yes, the purpose is to remove this heavy error message. This should be an error level, if guest actually set a bad base address, error message should be pop up.
>

If as you said, this could be a state also from host state, then not necessary to be taken as error.
Just error return for dmabuf user as it's not ready.

> Qemu are probing dma buf info at each qemu display refresh. Before guest driver enable the plane, its initial state should be disabled, then intel_vgpu_decode_primary/cursor/sprite_plane could return early, and avoid to check the plane base address. Finally error message will disappear before guest driver enable the plane.
> >
> > >
> > > This patch disable primary/sprite/cursor plane at vdisplay
> > > initialization to remove the above error.
> > >
> > 
> > I think the display driver shouldn't have any assumption on plane enable, so
> > this is not a must.
> > 
> > btw, 'vdisplay' is not a formal word for us, better write complete virtual
> > display.
> [Zhang, Xiong Y] ok, I will use full name "virtual display"
> > 
> > > Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/display.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/display.c
> > > b/drivers/gpu/drm/i915/gvt/display.c
> > > index dd96ffc..6d8180e 100644
> > > --- a/drivers/gpu/drm/i915/gvt/display.c
> > > +++ b/drivers/gpu/drm/i915/gvt/display.c
> > > @@ -169,6 +169,8 @@ static u8 dpcd_fix_data[DPCD_HEADER_SIZE] = {
> > > static void emulate_monitor_status_change(struct intel_vgpu *vgpu)  {
> > >  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> > > +	int pipe;
> > > +
> > >  	vgpu_vreg_t(vgpu, SDEISR) &= ~(SDE_PORTB_HOTPLUG_CPT |
> > >  			SDE_PORTC_HOTPLUG_CPT |
> > >  			SDE_PORTD_HOTPLUG_CPT);
> > > @@ -267,6 +269,14 @@ static void
> > emulate_monitor_status_change(struct intel_vgpu *vgpu)
> > >  	if (IS_BROADWELL(dev_priv))
> > >  		vgpu_vreg_t(vgpu, PCH_ADPA) &=
> > ~ADPA_CRT_HOTPLUG_MONITOR_MASK;
> > >
> > > +	/* Disable Primary/Sprite/Cursor plane */
> > > +	for_each_pipe(dev_priv, pipe) {
> > > +		vgpu_vreg_t(vgpu, DSPCNTR(pipe)) &= ~DISPLAY_PLANE_ENABLE;
> > > +		vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE;
> > > +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) &= ~CURSOR_MODE;
> > > +		vgpu_vreg_t(vgpu, CURCNTR(pipe)) |= CURSOR_MODE_DISABLE;
> > > +	}
> > > +
> > >  	vgpu_vreg_t(vgpu, PIPECONF(PIPE_A)) |= PIPECONF_ENABLE;  }
> > >
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> > 
> > --
> > Open Source Technology Center, Intel ltd.
> > 
> > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev