[Spice-devel] [PATCH qxl-wddm-dod 1/2] Fixing black-white screen in installation when qxl revision = 3
Sameeh Jubran
sameeh at daynix.com
Thu Oct 13 12:59:45 UTC 2016
On Thu, Oct 13, 2016 at 3:43 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> >
> > When qxl revision is 3, the vga driver is the one that is running. When
> > installing the driver the function VgaDevice::HWInit with displayInfo
> > structure that is zeroed out. The displayInfo should be intialized using
> > DxgkCbAcquirePostDisplayOwnership and thus it should be called before
> > calling HWInit.
> >
> > Please note that we can't just move the call to
> > "DxgkCbAcquirePostDisplayOwnership"
> > before calling HWInit as the m_Id isn't iniatilized for QxlDevice untill
> the
> > call
> > to HWinit is over.
> >
> > This patch fixies a bug similar to the one found here:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1202267
> > However this one occurs when installing the driver.
> >
>
> Minor typos
> intialized -> initialized
> iniatilized -> initialized
> untill -> until
> fixies -> fixes
>
> It's not clear to me the "However this one occurs when installing the
> driver."
> sentence. Do you mean that the bug refer to a problem similar but not
> occurring during installation?
>
Yes, exactly.
>
> > Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> > ---
> > qxldod/QxlDod.cpp | 68
> > +++++++++++++++++++++++++++----------------------------
> > qxldod/QxlDod.h | 2 ++
> > 2 files changed, 36 insertions(+), 34 deletions(-)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index 5cfff78..6ef57b8 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -124,7 +124,6 @@ NTSTATUS QxlDod::StartDevice(_In_ DXGK_START_INFO*
> > pDxgkStartInfo,
> > _Out_ ULONG* pNumberOfViews,
> > _Out_ ULONG* pNumberOfChildren)
> > {
> > - PHYSICAL_ADDRESS PhysicAddress;
> > PAGED_CODE();
> > QXL_ASSERT(pDxgkStartInfo != NULL);
> > QXL_ASSERT(pDxgkInterface != NULL);
> > @@ -176,32 +175,6 @@ NTSTATUS QxlDod::StartDevice(_In_ DXGK_START_INFO*
> > pDxgkStartInfo,
> > return Status;
> > }
> >
> > - PhysicAddress.QuadPart =
> > m_CurrentModes[0].DispInfo.PhysicAddress.QuadPart;
> > - if (m_pHWDevice->GetId() == 0)
> > - {
> > - Status =
> > m_DxgkInterface.DxgkCbAcquirePostDisplayOwnership(m_DxgkInterface.
> DeviceHandle,
> > &(m_CurrentModes[0].DispInfo));
> > - }
> > -
> > - if (!NT_SUCCESS(Status) )
> > - {
> > - DbgPrint(TRACE_LEVEL_ERROR, ("DxgkCbAcquirePostDisplayOwnership
> > failed with status 0x%X Width = %d\n",
> > - Status, m_CurrentModes[0].DispInfo.Width));
> > - return STATUS_UNSUCCESSFUL;
> > - }
> > -
> > - if (m_CurrentModes[0].DispInfo.Width == 0)
> > - {
> > - m_CurrentModes[0].DispInfo.Width = MIN_WIDTH_SIZE;
> > - m_CurrentModes[0].DispInfo.Height = MIN_HEIGHT_SIZE;
> > - m_CurrentModes[0].DispInfo.Pitch =
> > BPPFromPixelFormat(D3DDDIFMT_R8G8B8) / 8;
> > - m_CurrentModes[0].DispInfo.ColorFormat = D3DDDIFMT_R8G8B8;
> > - m_CurrentModes[0].DispInfo.TargetId = 0;
> > - if (PhysicAddress.QuadPart != 0L) {
> > - m_CurrentModes[0].DispInfo.PhysicAddress.QuadPart =
> > PhysicAddress.QuadPart;
> > - }
> > -
> > - }
> > -
> > *pNumberOfViews = MAX_VIEWS;
> > *pNumberOfChildren = MAX_CHILDREN;
> > m_Flags.DriverStarted = TRUE;
> > @@ -2488,12 +2461,6 @@ NTSTATUS
> > VgaDevice::GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
> >
> > m_CurrentMode = 0;
> > DbgPrint(TRACE_LEVEL_INFORMATION, ("m_ModeInfo = 0x%p,
> m_ModeNumbers =
> > 0x%p\n", m_ModeInfo, m_ModeNumbers));
> > - if (Width == 0 || Height == 0 || BitsPerPixel != VGA_BPP)
> > - {
> > - Width = MIN_WIDTH_SIZE;
> > - Height = MIN_HEIGHT_SIZE;
> > - BitsPerPixel = VGA_BPP;
> > - }
> > for (CurrentMode = 0, SuitableModeCount = 0;
> > CurrentMode < ModeCount;
> > CurrentMode++)
> > @@ -2621,7 +2588,7 @@ NTSTATUS VgaDevice::HWInit(PCM_RESOURCE_LIST
> pResList,
> > DXGK_DISPLAY_INFORMATION*
> >
> > DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> > UNREFERENCED_PARAMETER(pResList);
> > - UNREFERENCED_PARAMETER(pDispInfo);
> > + AcquireDisplayInfo(*(pDispInfo));
> > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> > return GetModeList(pDispInfo);
> > }
> > @@ -3391,6 +3358,7 @@ NTSTATUS QxlDevice::QxlInit(DXGK_
> DISPLAY_INFORMATION*
> > pDispInfo)
> > CreateMemSlots();
> > InitDeviceMemoryResources();
> > InitMonitorConfig();
> > + Status = AcquireDisplayInfo(*(pDispInfo));
> > return Status;
> > }
> >
> > @@ -4835,3 +4803,35 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
> > default: QXL_LOG_ASSERTION1("Unknown D3DDDIFORMAT 0x%I64x",
> Format);
> > return 0;
> > }
> > }
> > +
> > +NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_
> DISPLAY_INFORMATION&
> > DispInfo)
> > +{
> > + NTSTATUS Status = STATUS_SUCCESS;
> > + PHYSICAL_ADDRESS PhysicAddress;
> > + if (GetId() == 0)
> > + {
> > + Status = m_pQxlDod->AcquireDisplayInfo(DispInfo);
> > + }
> > +
> > + if (!NT_SUCCESS(Status))
> > + {
> > + DbgPrint(TRACE_LEVEL_ERROR, ("DxgkCbAcquirePostDisplayOwnership
> > failed with status 0x%X Width = %d\n",
> > + Status, DispInfo.Width));
>
> In this function you don't call DxgkCbAcquirePostDisplayOwnership but
> QxlDod::AcquireDisplayInfo which calls DxgkCbAcquirePostDisplayOwnership.
> I would either change the message or rename QxlDod::AcquireDisplayInfo
> to QxlDod::DxgkCbAcquirePostDisplayOwnership.
>
> > + return STATUS_UNSUCCESSFUL;
> > + }
> > + PhysicAddress.QuadPart = DispInfo.PhysicAddress.QuadPart;
> > +
> > + if (DispInfo.Width == 0)
> > + {
> > + DispInfo.Width = MIN_WIDTH_SIZE;
> > + DispInfo.Height = MIN_HEIGHT_SIZE;
> > + DispInfo.Pitch = BPPFromPixelFormat(D3DDDIFMT_R8G8B8) / 8;
> > + DispInfo.ColorFormat = D3DDDIFMT_R8G8B8;
> > + DispInfo.TargetId = 0;
> > + if (PhysicAddress.QuadPart != 0L)
> > + {
> > + DispInfo.PhysicAddress.QuadPart = PhysicAddress.QuadPart;
> > + }
> > + }
> > + return Status;
> > +}
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index bf16724..cd2032c 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -250,6 +250,7 @@ public:
> > virtual NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*
> > pSetPointerShape) = 0;
> > virtual NTSTATUS SetPointerPosition(_In_ CONST
> > DXGKARG_SETPOINTERPOSITION* pSetPointerPosition) = 0;
> > virtual NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap) = 0;
> > + NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo);
> > ULONG GetId(void) { return m_Id; }
> > protected:
> > virtual NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
> = 0;
> > @@ -704,6 +705,7 @@ public:
> > _In_
> > INT
> > PositionX,
> > _In_
> > INT
> > PositionY);
> > PDXGKRNL_INTERFACE GetDxgkInterface(void) { return
> &m_DxgkInterface;}
> > + NTSTATUS AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo) {
> return
> > m_DxgkInterface.DxgkCbAcquirePostDisplayOwnership(m_DxgkInterface.
> DeviceHandle,
> > &(DispInfo)); }
>
> you could use &DispInfo instead of &(DispInfo), feel free to ignore, just
> minor style.
>
> > private:
> > VOID CleanUp(VOID);
> > NTSTATUS CheckHardware();
>
> Frediano
>
--
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161013/b29edac0/attachment-0001.html>
More information about the Spice-devel
mailing list