On Thu, Oct 13, 2016 at 3:43 PM, Frediano Ziglio <fzig...@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 <sam...@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>.*
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
