<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> </head> <body text="#000000" bgcolor="#FFFFFF"> Thanks Ville and Daniel for for your response. I will try to come back with something later. thanks Lukasz <div class="moz-cite-prefix">On 21/12/2017 14:10, Daniel Vetter wrote: </div> <blockquote type="cite" cite="mid:20171221131040.GX26573@phenom.ffwll.local"> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> On Thu, Dec 21, 2017 at 02:46:55PM +0200, Ville Syrjälä wrote: > On Thu, Dec 21, 2017 at 12:10:08PM +0100, Lukasz Spintzyk wrote: > > Change-Id: I63dce004f8d3c5dc6a7c71070f1fab0707286ea5 > > Signed-off-by: Lukasz Spintzyk <a class="moz-txt-link-rfc2396E" href="mailto:lukasz.spintzyk@displaylink.com"><lukasz.spintzyk@displaylink.com></a> > > --- > > drivers/gpu/drm/drm_atomic.c | 10 ++++++++++ > > drivers/gpu/drm/drm_mode_config.c | 6 ++++++ > > drivers/gpu/drm/drm_plane.c | 1 + > > include/drm/drm_mode_config.h | 5 +++++ > > include/drm/drm_plane.h | 3 +++ > > 5 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index b76d49218cf1..cd3b4ed7b04c 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -759,6 +759,14 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, > > state->rotation = val; > > } else if (property == plane->zpos_property) { > > state->zpos = val; > > + } else if (property == config->dirty_rects_property) { > > + bool replaced = false; > > + int ret = drm_atomic_replace_property_blob_from_id(dev, > > + &state->dirty_blob, > > + val, > > + -1, > > + &replaced); > > + return ret; > > } else if (plane->funcs->atomic_set_property) { > > return plane->funcs->atomic_set_property(plane, state, > > property, val); > > @@ -818,6 +826,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > > *val = state->rotation; > > } else if (property == plane->zpos_property) { > > *val = state->zpos; > > + } else if (property == config->dirty_rects_property) { > > + *val = (state->dirty_blob) ? state->dirty_blob->base.id : 0; > > } else if (plane->funcs->atomic_get_property) { > > return plane->funcs->atomic_get_property(plane, state, property, val); > > } else { > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > > index bc5c46306b3d..d5f1021c6ece 100644 > > --- a/drivers/gpu/drm/drm_mode_config.c > > +++ b/drivers/gpu/drm/drm_mode_config.c > > @@ -293,6 +293,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) > > return -ENOMEM; > > dev->mode_config.prop_crtc_id = prop; > > > > + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, > > + "DIRTY_RECTS", 0); > > + if (!prop) > > + return -ENOMEM; > > + dev->mode_config.dirty_rects_property = prop; > > + > > prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC, > > "ACTIVE"); > > if (!prop) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index 37a93cdffb4a..add110f025e5 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -258,6 +258,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > > drm_object_attach_property(&plane->base, config->prop_src_y, 0); > > drm_object_attach_property(&plane->base, config->prop_src_w, 0); > > drm_object_attach_property(&plane->base, config->prop_src_h, 0); > > + drm_object_attach_property(&plane->base, config->dirty_rects_property, 0); > > } > > > > if (config->allow_fb_modifiers) > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > index e5f3b43014e1..65f64eb04c0c 100644 > > --- a/include/drm/drm_mode_config.h > > +++ b/include/drm/drm_mode_config.h > > @@ -599,6 +599,11 @@ struct drm_mode_config { > > * &drm_crtc. > > */ > > struct drm_property *prop_crtc_id; > > + /** > > + * @dirty_rects_property: Optional plane property to mark damaged > > + * regions on the plane framebuffer. > > What exactly would the blob contain? > > The comment seems to be implying these are in fb coordiantes as > opposed to plane crtc coordinates? Not sure which would be more > convenient. At least if they're fb coordinates then you probably > want some helpers to translate/rotate/scale those rects to the > crtc coordinates. Actual use depends on the driver/hw I suppose. Yeah I think we also should add a decoded state to the drm_plane_state, which has the full structure and all the details. </blockquote> Ok. Initially for model I was thinking to take struct drm_drawable_info with simple c style array of struct drm_clip_rect *rects in it. Do you think that something more complex will be needed? <blockquote type="cite" cite="mid:20171221131040.GX26573@phenom.ffwll.local"> And when we discussed this iirc we've identified a clear need for at least some drivers to deal in crtc dirty rectangles. I think the initial core support should include a helper which takes an atomic update for a given crtc, and converts all the plane dirty rectangles into crtc rectangles. Including any full-plane or full-crtc upgrades needed due to e.g. reposition, changed gamma, changed blendign/zpos or anything else really that would affect the entire plane respectively crtc. That would also provide a really good model for what damage actually means. </blockquote> Ok. <blockquote type="cite" cite="mid:20171221131040.GX26573@phenom.ffwll.local"> Plus ofc we need userspace for this, preferrably as a patch to something generic like weston or xfree86-video-modesetting. And an example kernel implementation. </blockquote> What kernel example would you think is the quickest/simplest for the proof of concept? I wanted to have something working on ChromeOS compositor first. Do you think it would satisfy need of userspace application? <blockquote type="cite" cite="mid:20171221131040.GX26573@phenom.ffwll.local"> Cheers, Daniel > > > + */ > > + struct drm_property *dirty_rects_property; > > /** > > * @prop_active: Default atomic CRTC property to control the active > > * state, which is the simplified implementation for DPMS in atomic > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > > index 8185e3468a23..7d45b164ccce 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -131,6 +131,9 @@ struct drm_plane_state { > > */ > > struct drm_crtc_commit *commit; > > > > + /* Optional blob property with damaged regions. */ > > + struct drm_property_blob *dirty_blob; > > + > > struct drm_atomic_state *state; > > }; > > > > -- > > 2.15.1 > > > > _______________________________________________ > > dri-devel mailing list > > <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> > > <a moz-do-not-send="true" href="https://lists.freedesktop.org/mailman/listinfo/dri-devel">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a> > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > dri-devel mailing list > <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> > <a moz-do-not-send="true" href="https://lists.freedesktop.org/mailman/listinfo/dri-devel">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a> -- Daniel Vetter Software Engineer, Intel Corporation <a moz-do-not-send="true" href="http://blog.ffwll.ch">http://blog.ffwll.ch</a> </blockquote> </body> </html>