From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Spintzyk Subject: Re: [RFC 0/3] drm: page-flip with damage Date: Tue, 10 Apr 2018 10:10:25 +0200 Message-ID: <5f3e1c8a-d2b9-41f9-46f6-2b7f8c736de8@displaylink.com> References: <1522885748-67122-1-git-send-email-drawat@vmware.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2035288827==" Return-path: Received: from eu-smtp-delivery-167.mimecast.com (eu-smtp-delivery-167.mimecast.com [146.101.78.167]) by gabe.freedesktop.org (Postfix) with ESMTPS id 673F8898C8 for ; Tue, 10 Apr 2018 08:17:11 +0000 (UTC) In-Reply-To: <1522885748-67122-1-git-send-email-drawat@vmware.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Deepak Rawat , "dri-devel@lists.freedesktop.org" , "thellstrom@vmware.com" , "syeh@vmware.com" Cc: "airlied@linux.ie" , "linux-graphics-maintainer@vmware.com" , "linux-kernel@vger.kernel.org" List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============2035288827== Content-Language: en-US Content-Type: multipart/alternative; boundary="------------4C2EA31473FAD28100AE6BBC" This is a multi-part message in MIME format. --------------4C2EA31473FAD28100AE6BBC Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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=20 are perfectly fine for DisplayLink case. Btw I have noticed that there is also a patch from Rob Clark that is=20 supplying helper for legacy dirtyfb callback. Maybe we should unify the naming of the rects. Here we have=20 "damage_clips", Clark's patch has 'dirty_rects' notion. On other hand existing legacy dirtyfb callback in drm_framebuffer_funcs=20 is also using 'clip_rects' :). Thanks =C5=81ukasz Spintzyk On 05/04/2018 01:49, Deepak Rawat wrote: > Hi All, > > This is extension to Lukasz Spintzyk earlier draft of damage interface=20 > for drm. > Bascially a new plane property is added called "DAMAGE_CLIPS" which is=20 > 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=20 > damage > clips in framebuffer, plane or crtc coordinates as need by driver > implementation. Finally a helper to reset damage in case need full=20 > update is > required. Drivers interested in page-flip with damage should call this=20 > 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=20 > 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=20 > driver use > cases and this can be modified in keeping other drivers need. > > Going forward driver implementation for vmwgfx and user-space=20 > 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=20 > ++++++++++++++++++++++++++++++++++++ > 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(+) > > --=20 > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel=20 > --------------4C2EA31473FAD28100AE6BBC Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

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

=C5=81ukasz 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

--------------4C2EA31473FAD28100AE6BBC-- --===============2035288827== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============2035288827==--