<div dir="ltr"><div dir="ltr"> </div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 18, 2019 at 4:02 PM Frediano Ziglio <<a href="mailto:fziglio@redhat.com">fziglio@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">By the way, the name of the function was not a regression, I acked and merged the patch.</blockquote><div> </div><div>It looks like the patch is not merged, BTW</div><div>If I'm mistaken, this means the repository migrated somewhere.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Maybe I'll think about a better name Frediano > On Thu, Jan 17, 2019 at 6:20 PM Frediano Ziglio < <a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a> > wrote: > > > > > > > <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1661147" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=1661147</a> > > > > If display miniport driver does not pass valid display info to > > > > the basic display driver on disable/uninstall, the basic > > > > display driver raises run-time error and stays with yellow > > > > bang. This behavior is specific to UEFI machines. > > > > Reference: > > > > <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/dispmprt/nc-dispmprt-dxgkddi_stop_device_and_release_post_display_ownership" rel="noreferrer" target="_blank">https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/dispmprt/nc-dispmprt-dxgkddi_stop_device_and_release_post_display_ownership</a> > > > > > > > > Signed-off-by: Yuri Benditovich < <a href="mailto:yuri.benditovich@daynix.com" target="_blank">yuri.benditovich@daynix.com</a> > > > > > --- > > > > qxldod/QxlDod.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++++++- > > > > qxldod/QxlDod.h | 6 +++++- > > > > 2 files changed, 51 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp > > > > index b72e12c..dea78e2 100755 > > > > --- a/qxldod/QxlDod.cpp > > > > +++ b/qxldod/QxlDod.cpp > > > > @@ -189,6 +189,12 @@ NTSTATUS QxlDod::StartDevice(_In_ DXGK_START_INFO* > > > > pDxgkStartInfo, > > > > return Status; > > > > } > > > > > > > > + DXGK_DISPLAY_INFORMATION *pInfo = &m_CurrentModes[0].DispInfo; > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Initial display info:\n", > > > > __FUNCTION__)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("ACPI/Target Id: %d:%d\n", > > > > pInfo->AcpiId, pInfo->TargetId)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("W/H/Pitch/Color:%d:%d:%d:%d\n", > > > > pInfo->Width, pInfo->Height, pInfo->Pitch, pInfo->ColorFormat)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("Physical address:%I64x\n", > > > > pInfo->PhysicAddress.QuadPart)); > > > > + > > > > Status = RegisterHWInfo(m_pHWDevice->GetId()); > > > > if (!NT_SUCCESS(Status)) > > > > { > > > > @@ -668,6 +674,12 @@ NTSTATUS > > > > QxlDod::StopDeviceAndReleasePostDisplayOwnership(_In_ D3DDDI_VIDEO_PRE > > > > m_pHWDevice->BlackOutScreen(&m_CurrentModes[SourceId]); > > > > > > > > *pDisplayInfo = m_CurrentModes[SourceId].DispInfo; > > > > + m_pHWDevice->GetFinalDisplayInfo(pDisplayInfo); > > > > + > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Final display info:\n", > > > > __FUNCTION__)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("ACPI/Target Id: %d:%d\n", > > > > pDisplayInfo->AcpiId, pDisplayInfo->TargetId)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("W/H/Pitch/Color:%d:%d:%d:%d\n", > > > > pDisplayInfo->Width, pDisplayInfo->Height, pDisplayInfo->Pitch, > > > > pDisplayInfo->ColorFormat)); > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("Physical address:%I64x\n", > > > > pDisplayInfo->PhysicAddress.QuadPart)); > > > > > > > > return StopDevice(); > > > > } > > > > @@ -3055,6 +3067,19 @@ NTSTATUS VgaDevice::Escape(_In_ CONST > > > DXGKARG_ESCAPE* > > > > pEscap) > > > > return STATUS_NOT_IMPLEMENTED; > > > > } > > > > > > > > +static BOOLEAN IsUefiMode() > > > > +{ > > > > + PAGED_CODE(); > > > > + UNICODE_STRING usName; > > > > + GUID guid = {}; > > > > + ULONG res, len = sizeof(res); > > > > + RtlInitUnicodeString(&usName, L"dummy"); > > > > + NTSTATUS status = ExGetFirmwareEnvironmentVariable(&usName, &guid, > > > &res, > > > > &len, NULL); > > > > + DbgPrint(TRACE_LEVEL_VERBOSE, ("%s, status %X\n", __FUNCTION__, > > > > status)); > > > > + // on UEFI system the status is STATUS_VARIABLE_NOT_FOUND > > > > + return status != STATUS_NOT_IMPLEMENTED; > > > > +} > > > > + > > > > QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod) > > > > { > > > > PAGED_CODE(); > > > > @@ -3068,6 +3093,8 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod) > > > > m_Pending = 0; > > > > m_PresentThread = NULL; > > > > m_bActive = FALSE; > > > > + m_bUefiMode = IsUefiMode(); > > > > + DbgPrint(TRACE_LEVEL_VERBOSE, ("%s, %s mode\n", __FUNCTION__, > > > > m_bUefiMode ? "UEFI" : "BIOS")); > > > > } > > > > > > > > QxlDevice::~QxlDevice(void) > > > > @@ -3331,6 +3358,17 @@ NTSTATUS > > > QxlDevice::SetPowerState(DEVICE_POWER_STATE > > > > DevicePowerState, DXGK_DISP > > > > return STATUS_SUCCESS; > > > > } > > > > > > > > +void QxlDevice::GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* > > > pDisplayInfo) > > > > +{ > > > > + PAGED_CODE(); > > > > + *pDisplayInfo = m_InitialDisplayInfo; > > > > + pDisplayInfo->TargetId = pDisplayInfo->AcpiId = 0; > > > > + if (!pDisplayInfo->PhysicAddress.QuadPart) > > > > + { > > > > + pDisplayInfo->PhysicAddress = m_RamPA; > > > > + } > > > > +} > > > > + > > > > NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST pResList, > > > > DXGK_DISPLAY_INFORMATION* pDispInfo) > > > > { > > > > PAGED_CODE(); > > > > @@ -3530,10 +3568,11 @@ NTSTATUS > > > QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* > > > > pDispInfo) > > > > DbgPrint(TRACE_LEVEL_ERROR, ("InitMonitorConfig failed with status > > > > 0x%X\n", Status)); > > > > return Status; > > > > } > > > > - Status = AcquireDisplayInfo(*(pDispInfo)); > > > > + Status = AcquireDisplayInfo(m_InitialDisplayInfo); > > > > if (NT_SUCCESS(Status)) > > > > { > > > > m_bActive = TRUE; > > > > + *pDispInfo = m_InitialDisplayInfo; > > > > Status = StartPresentThread(); > > > > } > > > > if (!NT_SUCCESS(Status)) { > > > > @@ -4728,6 +4767,11 @@ NTSTATUS QxlDevice::HWClose(void) > > > > { > > > > PAGED_CODE(); > > > > QxlClose(); > > > > + if (m_bUefiMode) > > > > + { > > > > + DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: Resetting the device\n", > > > > __FUNCTION__)); > > > > + WRITE_PORT_UCHAR((PUCHAR)(m_IoBase + QXL_IO_RESET), 0); > > > > + } > > > > UnmapMemory(); > > > > return STATUS_SUCCESS; > > > > } > > > > @@ -5143,6 +5187,7 @@ NTSTATUS > > > > HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf > > > > > > > > if (DispInfo.Width == 0) > > > > { > > > > + DbgPrint(TRACE_LEVEL_WARNING, ("QxlDod::AcquireDisplayInfo has zero > > > > width!\n")); > > > > DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8; > > > > DispInfo.Width = INITIAL_WIDTH; > > > > DispInfo.Height = INITIAL_HEIGHT; > > > > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h > > > > index eb6b78d..d889b9d 100755 > > > > --- a/qxldod/QxlDod.h > > > > +++ b/qxldod/QxlDod.h > > > > @@ -285,6 +285,7 @@ public: > > > > QXL_NON_PAGED virtual VOID VSyncInterruptPostProcess(_In_ > > > > PDXGKRNL_INTERFACE) = 0; > > > > virtual NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) { > > > > return STATUS_SUCCESS; } > > > > virtual NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) { > > > > return STATUS_SUCCESS; } > > > > + virtual void GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* > > > pDisplayInfo) > > > > {} > > > > > > > > ULONG GetModeCount(void) const {return m_ModeCount;} > > > > PVIDEO_MODE_INFORMATION GetModeInfo(UINT idx) {return &m_ModeInfo[idx];} > > > > @@ -555,7 +556,8 @@ public: > > > > NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* > > > > pSetPointerShape); > > > > NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* > > > > pSetPointerPosition); > > > > NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap); > > > > - BOOLEAN IsBIOSCompatible() { return FALSE; } > > > > + BOOLEAN IsBIOSCompatible() { return m_bUefiMode; } > > > So when is UEFI is BIOS compatible and when is BIOS is not BIOS compatible? > > > It looks a bit odd, I'm sure it works but is confusing. > > Indeed, this looks confusing, but in fact it is not. > When the system has a BIOS the BDD obtains video modes from the BIOS. > In MSFT headers the respective field has name 'SupportNonVGA', which does not > explain > anything at all, but used by class driver to understand whether the miniport > will provide on > leaving meaningful display info to BDD (Basic display driver), so the BDD can > continue with it. > For example, QXL driver in 'revision=3' mode uses video modes from the BIOS > and can report > exact BIOS-compatible mode when it leaves. So, the name 'BIOS compatible' in > the code > reflects the real ability to comply with BIOS-defined set of video modes. > QXL driver obtains video modes from the device and there is no compatibility > between device's video modes > and BIOS ones. So, in case of BIOS system for the QXL driver it is not so > simple to leave > the adapter in video mode compatible with BIOS mode list, so we do not make > such effort > (according to the documentation this is OK and certification test allows it). > When the system is UEFI, we do not have a choice, on driver stop we must > provide > such data to BDD. > If you have in mind better name for the field, please write it in response, I > will change it. > > > + void GetFinalDisplayInfo(DXGK_DISPLAY_INFORMATION* pDisplayInfo); > > > > protected: > > > > NTSTATUS GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo); > > > > QXLDrawable *PrepareBltBits (BLT_INFO* pDst, > > > > @@ -695,6 +697,8 @@ private: > > > > uint16_t m_DrawGeneration; > > > > HANDLE m_PresentThread; > > > > BOOLEAN m_bActive; > > > > + BOOLEAN m_bUefiMode; > > > > + DXGK_DISPLAY_INFORMATION m_InitialDisplayInfo; > > > > }; > > > > > > > > class QxlDod { > > > Frediano > </blockquote></div></div>