<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 26, 2016 at 6:20 PM, Frediano Ziglio <span dir="ltr"><<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">><br>
> Part of source image mapped by PresentDisplayOnly<br>
> should be big enough to cover all rectangles being<br>
> transferred.<br>
><br>
> Signed-off-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com">sameeh@daynix.com</a>><br>
> Signed-off-by: Dmitry Fleytman <<a href="mailto:dmitry@daynix.com">dmitry@daynix.com</a>><br>
> ---<br>
> qxldod/QxlDod.cpp | 28 +++++++++++++++++++++++-----<br>
> qxldod/QxlDod.h | 2 ++<br>
> 2 files changed, 25 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp<br>
> index ca3f8a3..b6c2494 100755<br>
> --- a/qxldod/QxlDod.cpp<br>
> +++ b/qxldod/QxlDod.cpp<br>
> @@ -3783,12 +3783,11 @@ QxlDevice::<wbr>ExecutePresentDisplayOnly(<br>
> ctx->Mdl = NULL;<br>
> ctx->DisplaySource = this;<br>
><br>
> - // Alternate between synch and asynch execution, for demonstrating<br>
> - // that a real hardware implementation can do either<br>
> -<br>
> + // Source bitmap is in user mode, must be locked under __try/__except<br>
> + // and mapped to kernel space before use.<br>
> {<br>
> - // Map Source into kernel space, as Blt will be executed by system<br>
> worker thread<br>
> - UINT sizeToMap = ctx->SrcPitch * ctx->SrcHeight;<br>
> + LONG maxHeight = GetMaxSourceMappingHeight(ctx-<wbr>>Moves,<br>
> ctx->NumMoves, ctx->DirtyRect, ctx->NumDirtyRects);<br>
> + UINT sizeToMap = ctx->SrcPitch * maxHeight;<br>
><br>
> PMDL mdl = IoAllocateMdl((PVOID)SrcAddr, sizeToMap, FALSE, FALSE,<br>
> NULL);<br>
> if(!mdl)<br>
> @@ -4699,6 +4698,25 @@ void QxlDevice::SetMonitorConfig(<wbr>QXLHead *<br>
> monitor_config)<br>
> AsyncIo(QXL_IO_MONITORS_<wbr>CONFIG_ASYNC, 0);<br>
> }<br>
><br>
> +LONG QxlDevice::<wbr>GetMaxSourceMappingHeight(<wbr>D3DKMT_MOVE_RECT* Moves, ULONG<br>
> NumMoves, RECT* DirtyRects, ULONG NumDirtyRects)<br>
> +{<br>
> + LONG maxHeight = 0;<br>
> + if (Moves != NULL) {<br>
> + for (UINT i = 0; i < NumMoves; i++) {<br>
> + POINT* pSourcePoint = &Moves[i].SourcePoint;<br>
> + RECT* pDestRect = &Moves[i].DestRect;<br>
> + maxHeight = MAX(maxHeight, pDestRect->bottom - pDestRect->top +<br>
> pSourcePoint->y);<br>
> + }<br>
> + }<br>
> + if (DirtyRects != NULL) {<br>
> + for (UINT i = 0; i < NumDirtyRects; i++) {<br>
> + RECT* pDirtyRect = &DirtyRects[i];<br>
> + maxHeight = MAX(maxHeight, pDirtyRect->bottom);<br>
> + }<br>
> + }<br>
> + return maxHeight;<br>
> +}<br>
> +<br>
<br>
</div></div>I saw it now. I was suggesting to remove the DirtyRects/Moves check for NULL<br>
as NumDirtyRects/NumMoves check should be enough.<br>
It's not clear however from <a href="https://msdn.microsoft.com/en-us/library/windows/hardware/hh451288(v=vs.85).aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-<wbr>us/library/windows/hardware/<wbr>hh451288(v=vs.85).aspx</a><br>
if is expected you should ignore NumDirtyRects if DirtyRects is NULL.<br>
It would be a weird specification having NumDirtyRects == 2 and DirtyRects == NULL.<br>
I would also use "const RECT&" but it's just style.<br></blockquote><div>Better safe than sorry :)</div><div>I agree with "const RECT&" </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
With this patch we optimize the mapping size shrinking the bottom part.<br>
Would be worth to shrink even to top part (although more complicated) ?<br></blockquote><div>This patch "expands" and doesn't shrink the mapping size as it calculates the max size</div><div>that should be mapped, this is the actual size that should be mapped in memory. This could</div><div>prevent rare BSODs when the mapped memory isn't enough for all of the rectangles.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> __declspec(code_seg("PAGE"))<br>
> NTSTATUS QxlDevice::Escape(_In_ CONST DXGKARG_ESCAPE* pEscape)<br>
> {<br>
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h<br>
> index 585b235..2f51328 100755<br>
> --- a/qxldod/QxlDod.h<br>
> +++ b/qxldod/QxlDod.h<br>
> @@ -645,6 +645,8 @@ private:<br>
> __declspec(code_seg("PAGE"))<br>
> void SetMonitorConfig(QXLHead* monitor_config);<br>
><br>
> + LONG static GetMaxSourceMappingHeight(<wbr>D3DKMT_MOVE_RECT* Moves, ULONG<br>
> NumMoves, RECT* DirtyRects, ULONG NumDirtyRects);<br>
> +<br>
> private:<br>
> PUCHAR m_IoBase;<br>
> BOOLEAN m_IoMapped;<br>
<br>
</span>I would use "static LONG", usually it's the order is used. And it's one<br>
C++ gotcha less (cfr "Creative Declaration-Specifier Ordering").<br></blockquote><div>Thanks for noting that! </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Frediano<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><font size="4" color="#0b5394" face="times new roman, serif">Respectfully,<br></font><div style="font-size:12.8px;color:rgb(136,136,136)"><font size="4" color="#0b5394" face="times new roman, serif"><b><i>Sameeh Jubran</i></b></font></div><div style="font-size:12.8px;color:rgb(136,136,136)"><i style="color:rgb(7,55,99);font-family:"times new roman",serif;font-size:large"><span style="line-height:15px"><a href="https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a" title="View public profile" name="UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_14e2c1de96f8c195_UNIQUE_ID_SafeHtmlFilter_SafeHtmlFilter_SafeHtmlFilter_webProfileURL" style="color:rgb(17,85,204);margin:0px;padding:0px;border-width:0px;outline:none;vertical-align:baseline;text-decoration:none" target="_blank">Linkedin</a></span></i><br></div><div style="font-size:12.8px;color:rgb(136,136,136)"><font size="4" face="times new roman, serif" color="#073763"><i>Junior Software Engineer @ <a href="http://www.daynix.com" target="_blank">Daynix</a>.</i></font></div></div></div></div></div></div></div>
</div></div>