RE: [PATCH 01/14] drm: add new plane property FB_DAMAGE_CLIPS to send damage during plane update

From: Deepak Singh Rawat <drawat@vmware.com>
To: Pekka Paalanen <ppaalanen@gmail.com>
Cc: "lukasz.spintzyk@displaylink.com"
	<lukasz.spintzyk@displaylink.com>,
	Thomas Hellstrom <thellstrom@vmware.com>,
	linux-graphics-maintainer <linux-graphics-maintainer@vmware.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: RE: [PATCH 01/14] drm: add new plane property FB_DAMAGE_CLIPS to send damage during plane update
Date: Tue, 11 Sep 2018 04:33:25 +0000	[thread overview]
Message-ID: <BYAPR05MB41817C807D93F294E09DFAF5BA040@BYAPR05MB4181.namprd05.prod.outlook.com> (raw)
In-Reply-To: <20180907122642.38588540@eldfell>

> 
> On Thu, 6 Sep 2018 21:36:51 +0000
> Deepak Singh Rawat <drawat@vmware.com> wrote:
> 
> > >
> > > - Why no input validation on the damage clips against the framebuffer
> > >   size? Ime not validating just leads to funny driver bugs down the road,
> > >   so what's the use-case you have in mind here?
> >
> > My motivation behind not informing user-space if damage is outside plane
> > src, sent during modeset(where it should be provided) or damage is outside
> > framebuffer coordinate (simply put damage clips are invalid) is that it's a
> > hint. The contract between kernel and user-space is simple, if damage
> > clips are valid, kernel might use them (but not always) otherwise they
> > will be ignored. Damage can be ignored deep in the stack (like network),
> > so all the input validation for nothing.
> >
> > Also, what all to consider in input validation ?
> > * clips are outside plane src
> > * damage clips sent during modeset should error ?
> > * any other properties.
> >
> > This will lead to a complex input validation during modeset check like
> > if some drivers are OK with damage while scaling whereas others
> > cannot support, should this error?
> >
> > However I am alright doing input validation if this becomes clear what
> > kind of validation to actually do. My understanding of drm core is limited.
> 
> Hi,
> 
> I think validating that the area of any clip rectangle does not extend
> beyond the framebuffer size would be good. A userspace violating that
> is arguably broken, so it would help catch userspace bugs.
> 
> I also would not assume that damage is irrelevant on modeset. Userspace
> does not always know if an update will be a modeset or not: drivers vary
> there, and userspace that just wants the update through will allow
> modesets always while still providing damage. Userspace could also
> change video mode timings or pixel format while keeping the resolution
> the same, and in that case damage could be meaningful to a driver.
> 
Thanks Pekka, for explanation. I agree clips outside framebuffer should
be errored and also it's not a lot for drm core. Also, will look into the
cases where drm core should clear the damage clips, like resolution
change, etc.

May be the better approach would be drm core just provide the helper
and driver takes care of driver specific scenarios.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel