RE: [RFC 0/3] drm: page-flip with damage

[Deepak Singh Rawat]

> 
> On Wed, Apr 04, 2018 at 04:49:05PM -0700, Deepak Rawat wrote:
> > Hi All,
> >
> > This is extension to Lukasz Spintzyk earlier draft of damage interface for
> drm.
> > Bascially a new plane property is added called "DAMAGE_CLIPS" which is
> simply
> > an array of drm_rect (exported to userspace as drm_mode_rect). The clips
> > represents damage in framebuffer coordinate of attached fb to the plane.
> >
> > Helper iterator is added to traverse the damage rectangles and get the
> damage
> > clips in framebuffer, plane or crtc coordinates as need by driver
> > implementation. Finally a helper to reset damage in case need full update is
> > required. Drivers interested in page-flip with damage should call this from
> > atomic_check hook.
> >
> > With the RFC for atomic implementation of dirtyfb ioctl I was thinking
> > should we need to consider dirty_fb flags, especially
> > DRM_MODE_FB_DIRTY_ANNOTATE_COPY to be passed with atomic
> > DAMAGE_CLIPS property blob? I didn't considered that untill now. If no
> driver
> > uses that in my opinion for simplicity this can be ignored?
> 
> Last time I've checked no driver nor userspace really used this. I'd drop
> it for the new uapi like you've done.

Thanks Daniel for the review.
Agreed, will drop dirty_fb flags unless someone has any objection.

> 
> > About overlaping of damage rectangles is also not finalized. This really
> > depends on driver specific implementation and can be left open-ended?
> 
> Overlapping is userspace being stupid imo :-) I guess we should say that
> drivers should at least not fall over overlapping rects, and if they can't
> handle them, either coalesce into one big rect, or split them on their own
> somehow.

I think the best is to suggest user-space to not send overlapped rects. But
as someone earlier suggested it is hard or I should say unnecessary check
to enforce this in driver.

> 
> > My knowledge is limited to vmwgfx so would like to hear about other
> driver use
> > cases and this can be modified in keeping other drivers need.
> >
> > Going forward driver implementation for vmwgfx and user-space
> implementation
> > of kmscube/weston will be next step to test the changes.
> 
> I think an implementation for -modesetting and weston would be perfect.
> Kmscube has very littel value for damage tracking purposes (it's not a
> full compositor, just renders new frame each scene). And for kms I'm not a
> fan of using vendor-specific drivers to demonstrate new generic uapi - way
> too big chances you're making some vendor driver specific hardcoded
> assumption that doesn't hold anywhere else. Also makes it harder for
> others to test-implement your uapi in their drivers.
> -Daniel

Agreed implementation in modesetting and Weston and for kernel part as
I read in another mail dirty_fb implementation is also a good use-case.

> 
> >
> > Thanks,
> > Deepak
> >
> > Deepak Rawat (2):
> >   drm: Add helper iterator functions to iterate over plane damage.
> >   drm: Add helper to validate damage during modeset_check
> >
> > Lukasz Spintzyk (1):
> >   drm: Add DAMAGE_CLIPS property to plane
> >
> >  drivers/gpu/drm/drm_atomic.c        |  42 +++++++++
> >  drivers/gpu/drm/drm_atomic_helper.c | 173
> ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_mode_config.c   |   5 ++
> >  drivers/gpu/drm/drm_plane.c         |  12 +++
> >  include/drm/drm_atomic_helper.h     |  41 +++++++++
> >  include/drm/drm_mode_config.h       |  15 ++++
> >  include/drm/drm_plane.h             |  16 ++++
> >  include/uapi/drm/drm_mode.h         |  15 ++++
> >  8 files changed, 319 insertions(+)
> >
> > --
> > 2.7.4
> >
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__blog.ffwll.ch&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=zOOG28inJK
> 0762SxAf-
> cyZdStnd2NQpRu98lJP2iYGw&m=CrefGziKAEk50MpTaNv64iDljHY7GesUlFHZc
> hWpiqI&s=gFk0ko5cD_cnIyPuZOrqyAwLsmTt9PiSDbWdHoi-8cU&e=