From kde-panel-devel  Tue May 30 09:30:21 2017
From: Oleg Chernovskiy <noreply () phabricator ! kde ! org>
Date: Tue, 30 May 2017 09:30:21 +0000
To: kde-panel-devel
Subject: D1230: GBM remote access support for KWin
Message-Id: <20170530093020.84211.F95E7989BD57235B () phabricator ! kde ! org>
X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=149613663122484
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--7b9879e4221649fe8a0599f90e951e0d"

--7b9879e4221649fe8a0599f90e951e0d
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="ascii"
Mime-Version: 1.0

Kanedias added inline comments.

INLINE COMMENTS

> davidedmundson wrote in drm_buffer.cpp:131
> If it's deferred it means someone else is doing the gbm_surface_release.
> 
> But we still need to set m_bo to nullptr. Otherwise it's potentially left dangling here after the RemoteAccessManager has deleted it.
> 
> (Alternately: if we changed DrmObjectPlane to store the buffers as QSharedPointers we could just keep a reference to the DrmBuffer in the RemoteAccessManager, which would be IMHO cleaner than doing low level GBM stuff there and having the data released in one of two places. I'll look into that)

I rewrote this part since this change and it doesn't touch buffers now, if you want to take a look at it prior to my resubmission, it's there:

https://gitlab.com/Kanedias/kwin/commit/9535f36bd09f8834be3773f25bf2075a720ba1c4

> davidedmundson wrote in remoteaccess_manager.h:49-50
> what's this signal for?

Forgot to get rid of it, thanks

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D1230

To: Kanedias, graesslin, davidedmundson
Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

--7b9879e4221649fe8a0599f90e951e0d
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset="ascii"
Mime-Version: 1.0

<table><tr><td style=3D"">Kanedias added inline comments.
</td><a style=3D"text-decoration: none; padding: 4px 8px; margin: 0 8px 8px=
; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; back=
ground-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1=
f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href=
=3D"https://phabricator.kde.org/D1230" rel=3D"noreferrer">View Revision</a>=
</tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style=3D"=
margin: 6px 0 12px 0;"><div style=3D"border: 1px solid #C7CCD9; border-radi=
us: 3px;"><div style=3D"padding: 0; background: #F7F7F7; border-color: #e3e=
4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style=
=3D"color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden=
;"><a style=3D"float: right; text-decoration: none;" href=3D"https://phabri=
cator.kde.org/D1230#inline-24724" rel=3D"noreferrer">View Inline</a><span s=
tyle=3D"color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in =
<span style=3D"color: #4b4d51; font-weight: bold;">drm_buffer.cpp:131</span=
></div>
<div style=3D"margin: 8px 0; padding: 0 12px; color: #74777D;"><p style=3D"=
padding: 0; margin: 8px;">If it&#039;s deferred it means someone else is do=
ing the gbm_surface_release.</p>

<p style=3D"padding: 0; margin: 8px;">But we still need to set m_bo to null=
ptr. Otherwise it&#039;s potentially left dangling here after the RemoteAcc=
essManager has deleted it.</p>

<p style=3D"padding: 0; margin: 8px;">(Alternately: if we changed DrmObject=
Plane to store the buffers as QSharedPointers we could just keep a referenc=
e to the DrmBuffer in the RemoteAccessManager, which would be IMHO cleaner =
than doing low level GBM stuff there and having the data released in one of=
 two places. I&#039;ll look into that)</p></div></div>
<div style=3D"margin: 8px 0; padding: 0 12px;"><p style=3D"padding: 0; marg=
in: 8px;">I rewrote this part since this change and it doesn&#039;t touch b=
uffers now, if you want to take a look at it prior to my resubmission, it&#=
039;s there:</p>

<p style=3D"padding: 0; margin: 8px;"><a href=3D"https://gitlab.com/Kanedia=
s/kwin/commit/9535f36bd09f8834be3773f25bf2075a720ba1c4" class=3D"remarkup-l=
ink" target=3D"_blank" rel=3D"noreferrer">https://gitlab.com/Kanedias/kwin/=
commit/9535f36bd09f8834be3773f25bf2075a720ba1c4</a></p></div></div><br /><d=
iv style=3D"border: 1px solid #C7CCD9; border-radius: 3px;"><div style=3D"p=
adding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid;=
 border-width: 0 0 1px 0; margin: 0;"><div style=3D"color: #74777d; backgro=
und: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style=3D"float: right=
; text-decoration: none;" href=3D"https://phabricator.kde.org/D1230#inline-=
24723" rel=3D"noreferrer">View Inline</a><span style=3D"color: #4b4d51; fon=
t-weight: bold;">davidedmundson</span> wrote in <span style=3D"color: #4b4d=
51; font-weight: bold;">remoteaccess_manager.h:49-50</span></div>
<div style=3D"margin: 8px 0; padding: 0 12px; color: #74777D;"><p style=3D"=
padding: 0; margin: 8px;">what&#039;s this signal for?</p></div></div>
<div style=3D"margin: 8px 0; padding: 0 12px;"><p style=3D"padding: 0; marg=
in: 8px;">Forgot to get rid of it, thanks</p></div></div></div></div></div>=
<br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div>=
<br /><div><strong>REVISION DETAIL</strong><div><a href=3D"https://phabrica=
tor.kde.org/D1230" rel=3D"noreferrer">https://phabricator.kde.org/D1230</a>=
</div></div><br /><div><strong>To: </strong>Kanedias, graesslin, davidedmun=
dson<br /><strong>Cc: </strong>davidedmundson, plasma-devel, ZrenBot, spsta=
rr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol=
, mart, lukas<br /></div>
--7b9879e4221649fe8a0599f90e951e0d--