D18570: EGLStream DRM Backend Initial Implementation

romangg added a comment.


  In D18570#415752 <https://phabricator.kde.org/D18570#415752>, @davidedmundson wrote:
  
  > > In this case it seems better to me to not write the EGLStreams support directly into the DRM Backend at all, but create a new EGLStream Backend on same level as DRM Backend. Th
  >
  > That sounds pointless. 
  >  There's less of a difference between EGL streams and GBM than there is between GBM and QPainter.
  
  
  GBM and QPainter both use the DrmBackend/DrmOutput methods as they are meant to be used including all atomic mode setting classes while EGLStream does everything internally bypassing the Drm methods. I'm not sure how you come to the conclusion EGLStreams and GBM would be more similar.
  
  > It would create a huge architectural problem given we create a platform before the scene so you'd at best have to duplicate the qpainter side.
  
  That's a good counter-argument though to having a separate backend. But everything in the DrmQPainterBackend besides the DrmDumbBuffer and DrmOutput pointers is independent of Drm code. But still the complexity grows, and therefore it might be better to not go down this road. Or maybe revisit it when multi-GPU is actually closer on the roadmap.
  
  Besides having it as a separate backend would be somewhat clunky looking at the backend auto selection code. There is currently an env var used for activating the EGLStream backend, which could be used in the auto backend selection as well, but in the end we maybe want to get rid of that variable. Is there already a plan for that by the way?
  
  Also I now notice again, that EglStreamerBackend class does also use the DrmOutput at one point to access its DrmCrtc and DrmPlane objects for getting the id of these kernel objects and then uses them in an EGL call. While this is not really much usage it makes it more reasonable again to have EGLStreams in the Drm backend.
  
  > In terms of this patch, I think it's pretty much good to go without any changes - and if there are any, I think that can be our responsibility rather than making the nvidia guy do it after they've done the brunt research work.
  
  I don't agree. We have no more responsibility to this patch than Nvidia. And I was not talking in my review comments about superficial stuff, but some base assumptions in this initial implementation. Getting these right is important. Your argument above was good in regards to not having a separate backend. But that does not change the issues I have with this solution here.
  
  I don't want the Drm backend get diluted by allowing hooks to be inserted, which break up well defined functional chains and bypass most important and defining functionality of the Drm backend. I'm open for discussing this more and also in helping to find a more uniform solution. But if there are no solutions in reasonable time to be found I'm open for making an exception under the current circumstances since at least we don't add lots of alternative code for the bypassed present function. That's in the driver.

REPOSITORY
  R108 KWin

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

To: ekurzinger, davidedmundson, graesslin, #kwin, romangg
Cc: wchilders, ognarb, abrahams, schernikov, guilhermesi, fredrik, cfeck, rapiteanu, romangg, hein, nicolasfella, zzag, kwin, jraleigh, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, alexeymin, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart