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