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

[Deepak Singh Rawat]

>Hi,
>
>Many thanks that you have picked it up.
>Unfortunately I didn't had time to work on it for a while.
>
>I am ok with what you have done, ihmo the review is going in good direction.
>I will try not to miss your update of it.
>From my side I can say that damage rects with frame-buffer coordinates are 
>perfectly fine for DisplayLink case.
>
>Btw I have noticed that there is also a patch from Rob Clark that is supplying 
>helper for legacy dirtyfb callback.
>Maybe we should unify the naming of the rects. Here we have "damage_clips", 
>Clark's patch has 'dirty_rects' notion.
>On other hand existing legacy dirtyfb callback in drm_framebuffer_funcs is 
>also using 'clip_rects' :).

The reason to name it damage is inspired by eglSwapBuffersWithDamageKHR
which user-space will be calling before submitting a page-flip. So it makes 
naming
consistent and in sync with user-space.

>
>Thanks
>
>Łukasz Spintzyk
>
>On 05/04/2018 01:49, 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?
>
>About overlaping of damage rectangles is also not finalized. This really
>depends on driver specific implementation and can be left open-ended?
>
>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.
>
>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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lukasz Spintzyk]

Hi,

Many thanks that you have picked it up.
Unfortunately I didn't had time to work on it for a while.

I am ok with what you have done, ihmo the review is going in good direction.
I will try not to miss your update of it.
From my side I can say that damage rects with frame-buffer coordinates 
are perfectly fine for DisplayLink case.


Btw I have noticed that there is also a patch from Rob Clark that is 
supplying helper for legacy dirtyfb callback.
Maybe we should unify the naming of the rects. Here we have 
"damage_clips", Clark's patch has 'dirty_rects' notion.
On other hand existing legacy dirtyfb callback in drm_framebuffer_funcs 
is also using 'clip_rects' :).



Thanks

Łukasz Spintzyk


On 05/04/2018 01:49, 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?

About overlaping of damage rectangles is also not finalized. This really
depends on driver specific implementation and can be left open-ended?

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.

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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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=DwIBAg=uilaK90D4TOVoH58JNXRgQ=zOOG28inJK
> 0762SxAf-
> cyZdStnd2NQpRu98lJP2iYGw=CrefGziKAEk50MpTaNv64iDljHY7GesUlFHZc
> hWpiqI=gFk0ko5cD_cnIyPuZOrqyAwLsmTt9PiSDbWdHoi-8cU=
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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.

> 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.

> 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

> 
> 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
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Deepak Rawat]

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?

About overlaping of damage rectangles is also not finalized. This really
depends on driver specific implementation and can be left open-ended?

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.

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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel