On Wed, Aug 7, 2019 at 12:36 PM Gerd Hoffmann kraxel@redhat.com wrote:
Hi,
Same for this, you're just upcasting to ttm_bo and then downcasting to gem_bo again ... I think just a series to roll out the existing gem helpers everywhere should work?
I don't think so. drm_gem_dumb_map_offset() calls drm_gem_create_mmap_offset(), which I think is not correct for ttm objects because ttm_bo_init() handles vma_node initialization.
More code to unify first? This should work exactly the same way for all gem based drivers I think ... Only tricky bit is making sure vmwgfx keeps working correctly.
Yea. Unifying on the gem way of doing things isn't going to work very well. We would have to keep the current way of doing things in the ttm code, wrapped into "if (ttm_bo_uses_embedded_gem_object()) { ... }", to not break vmwgfx.
So adding gem ttm helpers (where gem+ttm drivers can opt-in) looked like the better way of handling this to me ...
Ok I looked again, and your ttm version seems to exactly match drm_gem_dumb_map_offset(), which we almost called drm_gem_map_offset(). And could do that again by undoing that revert. So I'm not seeing how a generic version for this stuff here wouldn't also work for ttm ... Ofc if vmwgfx does something else they can keep their own specific dumb map_offset implementation.
What am I missing? -Daniel