[RFC] drm: Introduce max width and height properties for planes
On Mon, May 30, 2016 at 09:47:53AM -0400, Rob Clark wrote: > On Mon, May 30, 2016 at 5:32 AM, Daniel Vetter wrote: > > drm_hwcomposer (developed by Google) is a generic hwc implementation along > > the lines of weston and any other generic kms compositor. I think it makes > > a lot of sense to aim for a reasonable baseline API which works the same > > way everywhere. And that reasonable baseline abi imo includes that at > > least 1 plane can be used full-screen, without jumping through hooks. > > > > Because if you don't have that, then it's not just weston and android you > > need to patch up. But also > > - fbdev > > - any boot splash > > - any igt testcases (I'm starting to push hard to make those a requirement > > for kms drivers) > > - X > > - any other compositor, like mutter, kwin, ozone, ... > > I think it's not so bad. We fix the setcrtc helpers to do this for > legacy crtc path (which should pretty much cover most existing > compositors, X, splash, fbdev, etc). After all they already pick one > plane, it shouldn't be too hard to make them pick two. > > But for atomic, we make userspace deal with it, since things get more > complicated than what setcrtc/pageflip has to deal with. Afaiu there > are basically two atomic compositors (and the weston one isn't even > merged), so I think dealing with this in userspace is not so bad. What exactly is more complicated with doing this for full atomic? If you do it in userspace for setcrtc, plus fbdev, plus weston/ozone/drm_hwcomposer/ and maybe more, then that's already an impressive pile of copies of that logic. And I really think it should be pretty simple to type this up into a helper that works on any plane. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC] drm: Introduce max width and height properties for planes
On 05/25/2016 10:06 PM, Rob Clark wrote: > On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: >> On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >>> On 05/25/2016 12:40 PM, Daniel Vetter wrote: - Is the size/width really independent of e.g. rotation/pixel format/... Should it be the maximum that's possible under best circumstance (things could still fail), or the minimum that's guaranteed to work everwhere. This is the kind of stuff we need the userspace part for, too. >>> >>> Yeah, it isn't independent of these parameters. I'm not entirely sure >>> about this either. >>> >>> Does it make sense to impose a rule that the user first sets the >>> rotation/format plane properties, and only then read the maximum >>> width? I'm assuming user space hates such kind of stuff. >>> >>> If we use the 'best circumstance' max_width, we can first start >>> with a minimum number of planes that need to be grouped to achieve >>> the target mode. If that fails the atomic test, then we can try to >>> add one plane at a time, and reduce the width for each plane. >>> >>> If we use the minimum/'guaranteed to work' max_width, we'll get >>> a higher number of planes than needed for this mode. This would pass >>> the atomic test. We could then reduce a plane at a time and see when >>> we fail the atomic test. >>> >>> I guess we need to chose the one that's more probable to get right >>> the first time. Considering only pixel formats for now, the >>> minimum/'guaranteed to work' would map to the RGB formats. The best >>> circumstance ones would probably be the planar video formats. Since >>> we use RGB more often, the minimum one might make more sense. >>> >>> We could, of course, give the property a range of max widths to >>> confuse user space even more. >> >> An entirely different idea for cases where a simple hint property doesn't >> work (other ideas floating around are can_scale, to give a hint whether a >> plan can at least in theory up/downscale, or not at all), is that the >> kernel gives more specific hints about what it would like to change. >> >> So if userspace asks for a plane, but for the given pixel format it's too >> wide, the kernel could return a new proposed value for width. That would >> be super-flexible and could cover all kinds of use-case like rotation >> needing a specific tiling (fb_modifier) or specific pixel format, or >> specific stride. This would be great to have, but it sounds like a new class of ABI altogether, where you set a configuration, the kernel rejects it, but gives a hint about what it wants. Do we already have something in DRI that follows such a mechanism? >> >> For the case at hand there's even more worms: What about stride >> requirements? Afaik on some hw you just need to split the buffers into 2 >> planes, but can keep the wide stride (since the limit is the size of the >> linebuffers in the hw). On others you need to split the buffer itself into >> 2, because the plane hw can't cope with huge strides. Again might depend >> upon the pixel format. I assumed that hw always belonged to the first category. For the latter, the userspace strategy of allocating buffers would have to change itself. If we do decide to hide pipe virtualization in the kernel as you mentioned below, how would we manage plane hw that can't do huge strides. Would we need to copy the userspace populated buffer into two separate kernel buffers that fit the stride requirements? >> >> So in a way height/width is both too much information and not precise >> enough. Entirely different approches: >> - We just add a might_need_split_plane prop to crtcs where this might be >>needed. Userspace then gets to retry with split buffers if it doesn't >>work with a huge one. >> >> - When I discussed this with qualcom folks for msm we concluded that the >>simplest approach would be to hide this in the kernel. So if you have a >>too wide plane, and need 2 hw planes to scan it out, then do that >>transparently in the kernel. Of course this means that there will be 1 >>(or 3 if you need a 2x2 split) fewer planes available, but userspace >>needs to iteratively build up the plane config using ATOMIC_TEST anyway. > > Just fwiw, there are a few things that we will still end up > abstracting in the kernel by virtualizing the mapping between kms > planes and hw pipes. And the approach of weston atomic of > incrementally adding more planes w/ TESTONLY flag should work well for > that. (Let's hope the weston bits get upstream some day..) > > But exposing width limit avoids the one-plane to multiple-pipes case, > considerably simplifying things. And seemed like a generic enough > limit (iirc, it applies to omapdss and probably others), that it would > be cleaner to expose the limit to userspace. So there should be at > least a couple other drivers that could avoid virtualizing planes with > some help from userspace for this case. > > Regarding rotation, I'm n
[RFC] drm: Introduce max width and height properties for planes
On Mon, May 30, 2016 at 02:09:17PM +0530, Archit Taneja wrote: > > > On 05/25/2016 10:06 PM, Rob Clark wrote: > >On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: > >>On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: > >>>On 05/25/2016 12:40 PM, Daniel Vetter wrote: > - Is the size/width really independent of e.g. rotation/pixel format/... > Should it be the maximum that's possible under best circumstance > (things > could still fail), or the minimum that's guaranteed to work everwhere. > This is the kind of stuff we need the userspace part for, too. > >>> > >>>Yeah, it isn't independent of these parameters. I'm not entirely sure > >>>about this either. > >>> > >>>Does it make sense to impose a rule that the user first sets the > >>>rotation/format plane properties, and only then read the maximum > >>>width? I'm assuming user space hates such kind of stuff. > >>> > >>>If we use the 'best circumstance' max_width, we can first start > >>>with a minimum number of planes that need to be grouped to achieve > >>>the target mode. If that fails the atomic test, then we can try to > >>>add one plane at a time, and reduce the width for each plane. > >>> > >>>If we use the minimum/'guaranteed to work' max_width, we'll get > >>>a higher number of planes than needed for this mode. This would pass > >>>the atomic test. We could then reduce a plane at a time and see when > >>>we fail the atomic test. > >>> > >>>I guess we need to chose the one that's more probable to get right > >>>the first time. Considering only pixel formats for now, the > >>>minimum/'guaranteed to work' would map to the RGB formats. The best > >>>circumstance ones would probably be the planar video formats. Since > >>>we use RGB more often, the minimum one might make more sense. > >>> > >>>We could, of course, give the property a range of max widths to > >>>confuse user space even more. > >> > >>An entirely different idea for cases where a simple hint property doesn't > >>work (other ideas floating around are can_scale, to give a hint whether a > >>plan can at least in theory up/downscale, or not at all), is that the > >>kernel gives more specific hints about what it would like to change. > >> > >>So if userspace asks for a plane, but for the given pixel format it's too > >>wide, the kernel could return a new proposed value for width. That would > >>be super-flexible and could cover all kinds of use-case like rotation > >>needing a specific tiling (fb_modifier) or specific pixel format, or > >>specific stride. > > This would be great to have, but it sounds like a new class of ABI > altogether, where you set a configuration, the kernel rejects it, but > gives a hint about what it wants. Do we already have something in DRI > that follows such a mechanism? Yeah, we definitely don't want to start out with that. > >>For the case at hand there's even more worms: What about stride > >>requirements? Afaik on some hw you just need to split the buffers into 2 > >>planes, but can keep the wide stride (since the limit is the size of the > >>linebuffers in the hw). On others you need to split the buffer itself into > >>2, because the plane hw can't cope with huge strides. Again might depend > >>upon the pixel format. > > I assumed that hw always belonged to the first category. For the > latter, the userspace strategy of allocating buffers would have to > change itself. > > If we do decide to hide pipe virtualization in the kernel as you > mentioned below, how would we manage plane hw that can't do huge > strides. Would we need to copy the userspace populated buffer into > two separate kernel buffers that fit the stride requirements? I think at that point the kernel needs to make sure that the preferred mode is _not_ a mode that requires such a stride. And then userspace needs to piece stuff together on it's own. This is hw-mis-design on a level that I don't think makes much sense to abstract away ;-) But afaik the discussion here is just for hw where we might need 2 or 4 planes to scan out one logical buffer. > >>So in a way height/width is both too much information and not precise > >>enough. Entirely different approches: > >>- We just add a might_need_split_plane prop to crtcs where this might be > >> needed. Userspace then gets to retry with split buffers if it doesn't > >> work with a huge one. > >> > >>- When I discussed this with qualcom folks for msm we concluded that the > >> simplest approach would be to hide this in the kernel. So if you have a > >> too wide plane, and need 2 hw planes to scan it out, then do that > >> transparently in the kernel. Of course this means that there will be 1 > >> (or 3 if you need a 2x2 split) fewer planes available, but userspace > >> needs to iteratively build up the plane config using ATOMIC_TEST anyway. > > > >Just fwiw, there are a few things that we will still end up > >abstracting in the kernel by virtualizing the mapping between kms > >planes
[RFC] drm: Introduce max width and height properties for planes
On Mon, May 30, 2016 at 10:38 AM, Daniel Vetter wrote: > On Mon, May 30, 2016 at 09:47:53AM -0400, Rob Clark wrote: >> On Mon, May 30, 2016 at 5:32 AM, Daniel Vetter wrote: >> > drm_hwcomposer (developed by Google) is a generic hwc implementation along >> > the lines of weston and any other generic kms compositor. I think it makes >> > a lot of sense to aim for a reasonable baseline API which works the same >> > way everywhere. And that reasonable baseline abi imo includes that at >> > least 1 plane can be used full-screen, without jumping through hooks. >> > >> > Because if you don't have that, then it's not just weston and android you >> > need to patch up. But also >> > - fbdev >> > - any boot splash >> > - any igt testcases (I'm starting to push hard to make those a requirement >> > for kms drivers) >> > - X >> > - any other compositor, like mutter, kwin, ozone, ... >> >> I think it's not so bad. We fix the setcrtc helpers to do this for >> legacy crtc path (which should pretty much cover most existing >> compositors, X, splash, fbdev, etc). After all they already pick one >> plane, it shouldn't be too hard to make them pick two. >> >> But for atomic, we make userspace deal with it, since things get more >> complicated than what setcrtc/pageflip has to deal with. Afaiu there >> are basically two atomic compositors (and the weston one isn't even >> merged), so I think dealing with this in userspace is not so bad. > > What exactly is more complicated with doing this for full atomic? If you > do it in userspace for setcrtc, plus fbdev, plus s/userspace/kernel/ (for setcrtc/pageflip) > weston/ozone/drm_hwcomposer/ and maybe more, then that's already an > impressive pile of copies of that logic. And I really think it should be > pretty simple to type this up into a helper that works on any plane. For legacy setcrtc/pageflip, I think we just change crtc->primary into an array of N (where N is worst case # of planes). It is a scenario where you don't have to care about scaling, or really other planes in use, and a static association of planes to crtc's should be fine. And we wouldn't expose the additional primaries to userspace so no conflicts to worry about.. Seems like a fairly small change in the atomic helpers, vs having to invent virtual planes mapping to physical planes, etc. BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[RFC] drm: Introduce max width and height properties for planes
On Mon, May 30, 2016 at 5:32 AM, Daniel Vetter wrote: > On Mon, May 30, 2016 at 02:09:17PM +0530, Archit Taneja wrote: >> >> >> On 05/25/2016 10:06 PM, Rob Clark wrote: >> >On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: >> >>On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >> >>>On 05/25/2016 12:40 PM, Daniel Vetter wrote: >> - Is the size/width really independent of e.g. rotation/pixel format/... >> Should it be the maximum that's possible under best circumstance >> (things >> could still fail), or the minimum that's guaranteed to work everwhere. >> This is the kind of stuff we need the userspace part for, too. >> >>> >> >>>Yeah, it isn't independent of these parameters. I'm not entirely sure >> >>>about this either. >> >>> >> >>>Does it make sense to impose a rule that the user first sets the >> >>>rotation/format plane properties, and only then read the maximum >> >>>width? I'm assuming user space hates such kind of stuff. >> >>> >> >>>If we use the 'best circumstance' max_width, we can first start >> >>>with a minimum number of planes that need to be grouped to achieve >> >>>the target mode. If that fails the atomic test, then we can try to >> >>>add one plane at a time, and reduce the width for each plane. >> >>> >> >>>If we use the minimum/'guaranteed to work' max_width, we'll get >> >>>a higher number of planes than needed for this mode. This would pass >> >>>the atomic test. We could then reduce a plane at a time and see when >> >>>we fail the atomic test. >> >>> >> >>>I guess we need to chose the one that's more probable to get right >> >>>the first time. Considering only pixel formats for now, the >> >>>minimum/'guaranteed to work' would map to the RGB formats. The best >> >>>circumstance ones would probably be the planar video formats. Since >> >>>we use RGB more often, the minimum one might make more sense. >> >>> >> >>>We could, of course, give the property a range of max widths to >> >>>confuse user space even more. >> >> >> >>An entirely different idea for cases where a simple hint property doesn't >> >>work (other ideas floating around are can_scale, to give a hint whether a >> >>plan can at least in theory up/downscale, or not at all), is that the >> >>kernel gives more specific hints about what it would like to change. >> >> >> >>So if userspace asks for a plane, but for the given pixel format it's too >> >>wide, the kernel could return a new proposed value for width. That would >> >>be super-flexible and could cover all kinds of use-case like rotation >> >>needing a specific tiling (fb_modifier) or specific pixel format, or >> >>specific stride. >> >> This would be great to have, but it sounds like a new class of ABI >> altogether, where you set a configuration, the kernel rejects it, but >> gives a hint about what it wants. Do we already have something in DRI >> that follows such a mechanism? > > Yeah, we definitely don't want to start out with that. > >> >>For the case at hand there's even more worms: What about stride >> >>requirements? Afaik on some hw you just need to split the buffers into 2 >> >>planes, but can keep the wide stride (since the limit is the size of the >> >>linebuffers in the hw). On others you need to split the buffer itself into >> >>2, because the plane hw can't cope with huge strides. Again might depend >> >>upon the pixel format. >> >> I assumed that hw always belonged to the first category. For the >> latter, the userspace strategy of allocating buffers would have to >> change itself. >> >> If we do decide to hide pipe virtualization in the kernel as you >> mentioned below, how would we manage plane hw that can't do huge >> strides. Would we need to copy the userspace populated buffer into >> two separate kernel buffers that fit the stride requirements? > > I think at that point the kernel needs to make sure that the preferred > mode is _not_ a mode that requires such a stride. And then userspace needs > to piece stuff together on it's own. This is hw-mis-design on a level that > I don't think makes much sense to abstract away ;-) > > But afaik the discussion here is just for hw where we might need 2 or 4 > planes to scan out one logical buffer. right, if you need userspace to composite split buffers, you've already lost.. >> >>So in a way height/width is both too much information and not precise >> >>enough. Entirely different approches: >> >>- We just add a might_need_split_plane prop to crtcs where this might be >> >> needed. Userspace then gets to retry with split buffers if it doesn't >> >> work with a huge one. >> >> >> >>- When I discussed this with qualcom folks for msm we concluded that the >> >> simplest approach would be to hide this in the kernel. So if you have a >> >> too wide plane, and need 2 hw planes to scan it out, then do that >> >> transparently in the kernel. Of course this means that there will be 1 >> >> (or 3 if you need a 2x2 split) fewer planes available, but userspa
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 7:57 PM, Rob Clark wrote: >> As far as virtualizing the resources goes. I think that would need a >> whole new API. Or at least a separate set of objects perhaps. I'm too >> lazy to dig up all the old arguments now, but I'm pretty sure there >> were many. If this would be done, I suspect the only sane way to do it >> would be to just have a hwc implementation in the kernel. As in user >> space would pass in the desired configuration, and the kernel would >> assign resources as best it can and return the result back to userspace. > > actually, without introducing a completely new/different uapi, it > should work pretty reasonably with the way weston atomic works, > incrementally adding planes in TESTONLY steps until it fails. I > suspect on some more creative hw (msm is prime candidate) we'll still > see custom hwc implementations to squeeze out the last bit of > performance/power. But with the approach that weston uses a generic > userspace should still work pretty well on that hw. So summary from my points on the m-l: - Whether we do this in all drivers that need this for dual-pipe 4k, or in all compositors is probably similar amounts of work. The upside of doing it in drivers is that it's closer to the metal, avoid midlayer-like mistakes in trying to funnel tons of stuff between the top and bottom layers through drm core, and still failing in corner cases. Drivers can simply fix things up if they're in control. - For a bunch of reasons we (well Ville is really big on those) want to add clipped_src/dst rectangles to drm_plane_state. That's a bit of churn, but it's per-driver opt-in churn, so manageable. Once we have clipped rectangles and so fully decoupled the hw state from the uabi state it's easy to clip a plane to a sub-pipe instead of the entire virtual area of the crtc. - Once we can clip planes we can add a bit of helper magic to steal additional&unused planes for use as the 2nd/3rd/4th plane scanning out one logical surface (drm_buffer) attached to a logical plane (drm_plane). Add a helper on top which splits a 4/5k screen at a passed in limit, and this boils down to a few lines in drivers. While still giving the drivers full control over all the details, if they want to. - Really assigning resources within the driver while exposing whether there's enough to userspace using TEST_ONLY is the point of atomic. Doesn't matter whether it's fifo space, scalers, wizzbang gizmo A, B or C or this thing called "scanout engine". If your hw needs more than one of those to get a basic job done (fullscreen primary plane) then it's imo the drivers job to juggle all the internal resources around. KMS is supposed to work from workstation gpus down to soc in a somewhat uniform way, even on crappy hw. Summary: Still firmly in the "virtualize it!" camp ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 12:36:53PM -0400, Rob Clark wrote: > On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: > > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: > >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: > >> >- Is the size/width really independent of e.g. rotation/pixel format/... > >> > Should it be the maximum that's possible under best circumstance > >> > (things > >> > could still fail), or the minimum that's guaranteed to work everwhere. > >> > This is the kind of stuff we need the userspace part for, too. > >> > >> Yeah, it isn't independent of these parameters. I'm not entirely sure > >> about this either. > >> > >> Does it make sense to impose a rule that the user first sets the > >> rotation/format plane properties, and only then read the maximum > >> width? I'm assuming user space hates such kind of stuff. > >> > >> If we use the 'best circumstance' max_width, we can first start > >> with a minimum number of planes that need to be grouped to achieve > >> the target mode. If that fails the atomic test, then we can try to > >> add one plane at a time, and reduce the width for each plane. > >> > >> If we use the minimum/'guaranteed to work' max_width, we'll get > >> a higher number of planes than needed for this mode. This would pass > >> the atomic test. We could then reduce a plane at a time and see when > >> we fail the atomic test. > >> > >> I guess we need to chose the one that's more probable to get right > >> the first time. Considering only pixel formats for now, the > >> minimum/'guaranteed to work' would map to the RGB formats. The best > >> circumstance ones would probably be the planar video formats. Since > >> we use RGB more often, the minimum one might make more sense. > >> > >> We could, of course, give the property a range of max widths to > >> confuse user space even more. > > > > An entirely different idea for cases where a simple hint property doesn't > > work (other ideas floating around are can_scale, to give a hint whether a > > plan can at least in theory up/downscale, or not at all), is that the > > kernel gives more specific hints about what it would like to change. > > > > So if userspace asks for a plane, but for the given pixel format it's too > > wide, the kernel could return a new proposed value for width. That would > > be super-flexible and could cover all kinds of use-case like rotation > > needing a specific tiling (fb_modifier) or specific pixel format, or > > specific stride. > > > > For the case at hand there's even more worms: What about stride > > requirements? Afaik on some hw you just need to split the buffers into 2 > > planes, but can keep the wide stride (since the limit is the size of the > > linebuffers in the hw). On others you need to split the buffer itself into > > 2, because the plane hw can't cope with huge strides. Again might depend > > upon the pixel format. > > > > So in a way height/width is both too much information and not precise > > enough. Entirely different approches: > > - We just add a might_need_split_plane prop to crtcs where this might be > > needed. Userspace then gets to retry with split buffers if it doesn't > > work with a huge one. > > > > - When I discussed this with qualcom folks for msm we concluded that the > > simplest approach would be to hide this in the kernel. So if you have a > > too wide plane, and need 2 hw planes to scan it out, then do that > > transparently in the kernel. Of course this means that there will be 1 > > (or 3 if you need a 2x2 split) fewer planes available, but userspace > > needs to iteratively build up the plane config using ATOMIC_TEST anyway. > > Just fwiw, there are a few things that we will still end up > abstracting in the kernel by virtualizing the mapping between kms > planes and hw pipes. And the approach of weston atomic of > incrementally adding more planes w/ TESTONLY flag should work well for > that. (Let's hope the weston bits get upstream some day..) > > But exposing width limit avoids the one-plane to multiple-pipes case, > considerably simplifying things. And seemed like a generic enough > limit (iirc, it applies to omapdss and probably others), that it would > be cleaner to expose the limit to userspace. So there should be at > least a couple other drivers that could avoid virtualizing planes with > some help from userspace for this case. > > Regarding rotation, I'm not 100% sure.. seems like we could just > document these as the un-rotated limits. If we really had to, we > could do some sort of dance where userspace sets rotation property on > an un-used plane, and then reads-back the current values of the > read-only prop's. But that seems awkward. I'm thinking all of this is doomed to fail. So right now people seem to want some kind of maximum size of the source viewport. What about the destination size? Is the max size for unscaled/scaled/smething else? Rotation/pixel format were already mentioned. How does this int
[RFC] drm: Introduce max width and height properties for planes
On 05/25/2016 12:40 PM, Daniel Vetter wrote: > On Wed, May 25, 2016 at 12:03:39PM +0530, Archit Taneja wrote: >> High resoultion modes (4K/5K) are supported by encoder/crtc hardware, but >> the backing hardware planes to scanout these buffers generally don't >> support such large widths. Two (or more) hardware planes are used in >> conjunction to scan out the entire width. > > "generally" seems to be a bit an overstatement. Most sensible hw has no > problem at all scanning that kind of stuff out ;-) > >> One way to support this is to virtualize the drm_plane objects that we >> expose to userspace, and let the kms driver internally figure out whether >> it needs to club multiple hardware pipes to run a particular mode. >> >> The other option is to represent a drm_plane as it is in hardware, and >> leave it to userspace use to manage multiple drm_planes for scanning >> out these modes. >> >> The advantage of the latter option is that the kms driver doesn't get >> too complicated. It doesn't need to do the decision making on how to >> distibute the hardware resources among the drm_planes. It also allows >> userspace to continue doing some of the heavy lifting (figure out >> the most power efficient combination of pipes, estimate bandwidth >> consumption etc) which would have to be moved to the kernel if we >> virtualized planes. >> >> In order to achieve the latter, a drm_plane should expose an immutable >> property to userspace describing the maximum width and height it can >> support. Userspace can then prepare the drm_planes accordingly and >> commit the state. This patch introduces these properties. >> >> The main disadvantage is that legacy userspace (i.e. calling SetCrtc >> on a high resolution framebuffer) wouldn't work. This, on the other >> hand, wouldn't be a problem if we virtualized the planes and manipulated >> the hardware pipes in the kenrel. One way around this is to fail large >> resolution modes when using legacy setcrtc, or introduce some minimal >> helpers in the kernel that automatically use multiple drm_planes when >> we try to set such a mode. A solution to this issue isn't a part of the >> RFC yet. >> >> We're looking for feedback here and wondering whether this is the right >> way to go or not. > > So just this week there was a bit a discussion on irc (and I haven't > gotten around to typing the writeup and asking everyone for feedback on > it) on new properties. Super-short summary: > > - new properties are new abi like anything else > > - drm wants a userspace demonstration vehicle to prove new abi which is: >open source, patches reviewed & tested by respective canonical upstream >(so not a vendor fork or something like that). Testcases are explicitly >not enough, it needs to be the real deal. > > - for props specifically which are meant to be used/interpreted by >compositor this means a patch for drm_hwcomposer, one of the many >wayland compositors there are (weston, ozone, mutter), or a patch for >xfree86-video-modesetting. Other userspace thingies are probably a bit >too much fringe to count. > > - this isn't really new, but thus far arm simply sailed in the shadow of >intel doing all that hard work. Now some arm drivers start to pull >ahead, adding new ABI that's not yet proven by i915 folks, and with >success comes a bit more responsibility. That sounds fair enough. > > For the interface itself just a few questions: > - We should make the cursor size stuff obselete by this, and instead first >look up the size limits of the cursor plane, before checking those >legacy limits. Yeah, I guess querying the DRM_CAP_CURSOR_WIDTH/HEIGHT caps would become obsolete. > - Is the size/width really independent of e.g. rotation/pixel format/... >Should it be the maximum that's possible under best circumstance (things >could still fail), or the minimum that's guaranteed to work everwhere. >This is the kind of stuff we need the userspace part for, too. Yeah, it isn't independent of these parameters. I'm not entirely sure about this either. Does it make sense to impose a rule that the user first sets the rotation/format plane properties, and only then read the maximum width? I'm assuming user space hates such kind of stuff. If we use the 'best circumstance' max_width, we can first start with a minimum number of planes that need to be grouped to achieve the target mode. If that fails the atomic test, then we can try to add one plane at a time, and reduce the width for each plane. If we use the minimum/'guaranteed to work' max_width, we'll get a higher number of planes than needed for this mode. This would pass the atomic test. We could then reduce a plane at a time and see when we fail the atomic test. I guess we need to chose the one that's more probable to get right the first time. Considering only pixel formats for now, the minimum/'guaranteed to work' would map to the RGB formats. The best circumstance ones would probably b
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote:
> On 05/25/2016 12:40 PM, Daniel Vetter wrote:
> >- Is the size/width really independent of e.g. rotation/pixel format/...
> > Should it be the maximum that's possible under best circumstance (things
> > could still fail), or the minimum that's guaranteed to work everwhere.
> > This is the kind of stuff we need the userspace part for, too.
>
> Yeah, it isn't independent of these parameters. I'm not entirely sure
> about this either.
>
> Does it make sense to impose a rule that the user first sets the
> rotation/format plane properties, and only then read the maximum
> width? I'm assuming user space hates such kind of stuff.
>
> If we use the 'best circumstance' max_width, we can first start
> with a minimum number of planes that need to be grouped to achieve
> the target mode. If that fails the atomic test, then we can try to
> add one plane at a time, and reduce the width for each plane.
>
> If we use the minimum/'guaranteed to work' max_width, we'll get
> a higher number of planes than needed for this mode. This would pass
> the atomic test. We could then reduce a plane at a time and see when
> we fail the atomic test.
>
> I guess we need to chose the one that's more probable to get right
> the first time. Considering only pixel formats for now, the
> minimum/'guaranteed to work' would map to the RGB formats. The best
> circumstance ones would probably be the planar video formats. Since
> we use RGB more often, the minimum one might make more sense.
>
> We could, of course, give the property a range of max widths to
> confuse user space even more.
An entirely different idea for cases where a simple hint property doesn't
work (other ideas floating around are can_scale, to give a hint whether a
plan can at least in theory up/downscale, or not at all), is that the
kernel gives more specific hints about what it would like to change.
So if userspace asks for a plane, but for the given pixel format it's too
wide, the kernel could return a new proposed value for width. That would
be super-flexible and could cover all kinds of use-case like rotation
needing a specific tiling (fb_modifier) or specific pixel format, or
specific stride.
For the case at hand there's even more worms: What about stride
requirements? Afaik on some hw you just need to split the buffers into 2
planes, but can keep the wide stride (since the limit is the size of the
linebuffers in the hw). On others you need to split the buffer itself into
2, because the plane hw can't cope with huge strides. Again might depend
upon the pixel format.
So in a way height/width is both too much information and not precise
enough. Entirely different approches:
- We just add a might_need_split_plane prop to crtcs where this might be
needed. Userspace then gets to retry with split buffers if it doesn't
work with a huge one.
- When I discussed this with qualcom folks for msm we concluded that the
simplest approach would be to hide this in the kernel. So if you have a
too wide plane, and need 2 hw planes to scan it out, then do that
transparently in the kernel. Of course this means that there will be 1
(or 3 if you need a 2x2 split) fewer planes available, but userspace
needs to iteratively build up the plane config using ATOMIC_TEST anyway.
If possible for your hw I'm heavily leaning towards this last approach. If
fits entirely into the current atomic design, and all the complexity is
restricted to your driver (you need to have some allocation map between
drm planes and real hw planes, but that's it).
Thoughts?
-Daniel
>
> Archit
>
> >
> >Cheers, Daniel
> >
> >>Signed-off-by: Archit Taneja
> >>---
> >> drivers/gpu/drm/drm_atomic.c | 9 +
> >> drivers/gpu/drm/drm_crtc.c | 38 ++
> >> include/drm/drm_crtc.h | 6 ++
> >> 3 files changed, 53 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >>index 8ee1db8..fded22a 100644
> >>--- a/drivers/gpu/drm/drm_atomic.c
> >>+++ b/drivers/gpu/drm/drm_atomic.c
> >>@@ -839,6 +839,15 @@ static int drm_atomic_plane_check(struct drm_plane
> >>*plane,
> >>return -ERANGE;
> >>}
> >>
> >>+ if (plane->max_width && (state->src_w >> 16) > plane->max_width ||
> >>+ plane->max_height && (state->src_h >> 16) > plane->max_height) {
> >>+ DRM_DEBUG_ATOMIC("Invalid source width/height "
> >>+"%u.%06ux%u.%06u\n",
> >>+state->src_w >> 16, ((state->src_w & 0x) *
> >>15625) >> 10,
> >>+state->src_h >> 16, ((state->src_h & 0x) *
> >>15625) >> 10);
> >>+ return -ERANGE;
> >>+ }
> >>+
> >>fb_width = state->fb->width << 16;
> >>fb_height = state->fb->height << 16;
> >>
> >>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> >>index e08f962..f2d3b92 100644
> >>--- a/drivers/gpu/drm/d
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 1:20 PM, Ville Syrjälä wrote: > On Wed, May 25, 2016 at 12:36:53PM -0400, Rob Clark wrote: >> On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: >> > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >> >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: >> >> >- Is the size/width really independent of e.g. rotation/pixel format/... >> >> > Should it be the maximum that's possible under best circumstance >> >> > (things >> >> > could still fail), or the minimum that's guaranteed to work everwhere. >> >> > This is the kind of stuff we need the userspace part for, too. >> >> >> >> Yeah, it isn't independent of these parameters. I'm not entirely sure >> >> about this either. >> >> >> >> Does it make sense to impose a rule that the user first sets the >> >> rotation/format plane properties, and only then read the maximum >> >> width? I'm assuming user space hates such kind of stuff. >> >> >> >> If we use the 'best circumstance' max_width, we can first start >> >> with a minimum number of planes that need to be grouped to achieve >> >> the target mode. If that fails the atomic test, then we can try to >> >> add one plane at a time, and reduce the width for each plane. >> >> >> >> If we use the minimum/'guaranteed to work' max_width, we'll get >> >> a higher number of planes than needed for this mode. This would pass >> >> the atomic test. We could then reduce a plane at a time and see when >> >> we fail the atomic test. >> >> >> >> I guess we need to chose the one that's more probable to get right >> >> the first time. Considering only pixel formats for now, the >> >> minimum/'guaranteed to work' would map to the RGB formats. The best >> >> circumstance ones would probably be the planar video formats. Since >> >> we use RGB more often, the minimum one might make more sense. >> >> >> >> We could, of course, give the property a range of max widths to >> >> confuse user space even more. >> > >> > An entirely different idea for cases where a simple hint property doesn't >> > work (other ideas floating around are can_scale, to give a hint whether a >> > plan can at least in theory up/downscale, or not at all), is that the >> > kernel gives more specific hints about what it would like to change. >> > >> > So if userspace asks for a plane, but for the given pixel format it's too >> > wide, the kernel could return a new proposed value for width. That would >> > be super-flexible and could cover all kinds of use-case like rotation >> > needing a specific tiling (fb_modifier) or specific pixel format, or >> > specific stride. >> > >> > For the case at hand there's even more worms: What about stride >> > requirements? Afaik on some hw you just need to split the buffers into 2 >> > planes, but can keep the wide stride (since the limit is the size of the >> > linebuffers in the hw). On others you need to split the buffer itself into >> > 2, because the plane hw can't cope with huge strides. Again might depend >> > upon the pixel format. >> > >> > So in a way height/width is both too much information and not precise >> > enough. Entirely different approches: >> > - We just add a might_need_split_plane prop to crtcs where this might be >> > needed. Userspace then gets to retry with split buffers if it doesn't >> > work with a huge one. >> > >> > - When I discussed this with qualcom folks for msm we concluded that the >> > simplest approach would be to hide this in the kernel. So if you have a >> > too wide plane, and need 2 hw planes to scan it out, then do that >> > transparently in the kernel. Of course this means that there will be 1 >> > (or 3 if you need a 2x2 split) fewer planes available, but userspace >> > needs to iteratively build up the plane config using ATOMIC_TEST anyway. >> >> Just fwiw, there are a few things that we will still end up >> abstracting in the kernel by virtualizing the mapping between kms >> planes and hw pipes. And the approach of weston atomic of >> incrementally adding more planes w/ TESTONLY flag should work well for >> that. (Let's hope the weston bits get upstream some day..) >> >> But exposing width limit avoids the one-plane to multiple-pipes case, >> considerably simplifying things. And seemed like a generic enough >> limit (iirc, it applies to omapdss and probably others), that it would >> be cleaner to expose the limit to userspace. So there should be at >> least a couple other drivers that could avoid virtualizing planes with >> some help from userspace for this case. >> >> Regarding rotation, I'm not 100% sure.. seems like we could just >> document these as the un-rotated limits. If we really had to, we >> could do some sort of dance where userspace sets rotation property on >> an un-used plane, and then reads-back the current values of the >> read-only prop's. But that seems awkward. > > I'm thinking all of this is doomed to fail. So right now people seem to > want some kind of maximum size of the source viewport. Wh
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 9:12 AM, Daniel Vetter wrote: > On Wed, May 25, 2016 at 04:28:36PM +0530, Archit Taneja wrote: >> On 05/25/2016 12:40 PM, Daniel Vetter wrote: >> >- Is the size/width really independent of e.g. rotation/pixel format/... >> > Should it be the maximum that's possible under best circumstance (things >> > could still fail), or the minimum that's guaranteed to work everwhere. >> > This is the kind of stuff we need the userspace part for, too. >> >> Yeah, it isn't independent of these parameters. I'm not entirely sure >> about this either. >> >> Does it make sense to impose a rule that the user first sets the >> rotation/format plane properties, and only then read the maximum >> width? I'm assuming user space hates such kind of stuff. >> >> If we use the 'best circumstance' max_width, we can first start >> with a minimum number of planes that need to be grouped to achieve >> the target mode. If that fails the atomic test, then we can try to >> add one plane at a time, and reduce the width for each plane. >> >> If we use the minimum/'guaranteed to work' max_width, we'll get >> a higher number of planes than needed for this mode. This would pass >> the atomic test. We could then reduce a plane at a time and see when >> we fail the atomic test. >> >> I guess we need to chose the one that's more probable to get right >> the first time. Considering only pixel formats for now, the >> minimum/'guaranteed to work' would map to the RGB formats. The best >> circumstance ones would probably be the planar video formats. Since >> we use RGB more often, the minimum one might make more sense. >> >> We could, of course, give the property a range of max widths to >> confuse user space even more. > > An entirely different idea for cases where a simple hint property doesn't > work (other ideas floating around are can_scale, to give a hint whether a > plan can at least in theory up/downscale, or not at all), is that the > kernel gives more specific hints about what it would like to change. > > So if userspace asks for a plane, but for the given pixel format it's too > wide, the kernel could return a new proposed value for width. That would > be super-flexible and could cover all kinds of use-case like rotation > needing a specific tiling (fb_modifier) or specific pixel format, or > specific stride. > > For the case at hand there's even more worms: What about stride > requirements? Afaik on some hw you just need to split the buffers into 2 > planes, but can keep the wide stride (since the limit is the size of the > linebuffers in the hw). On others you need to split the buffer itself into > 2, because the plane hw can't cope with huge strides. Again might depend > upon the pixel format. > > So in a way height/width is both too much information and not precise > enough. Entirely different approches: > - We just add a might_need_split_plane prop to crtcs where this might be > needed. Userspace then gets to retry with split buffers if it doesn't > work with a huge one. > > - When I discussed this with qualcom folks for msm we concluded that the > simplest approach would be to hide this in the kernel. So if you have a > too wide plane, and need 2 hw planes to scan it out, then do that > transparently in the kernel. Of course this means that there will be 1 > (or 3 if you need a 2x2 split) fewer planes available, but userspace > needs to iteratively build up the plane config using ATOMIC_TEST anyway. Just fwiw, there are a few things that we will still end up abstracting in the kernel by virtualizing the mapping between kms planes and hw pipes. And the approach of weston atomic of incrementally adding more planes w/ TESTONLY flag should work well for that. (Let's hope the weston bits get upstream some day..) But exposing width limit avoids the one-plane to multiple-pipes case, considerably simplifying things. And seemed like a generic enough limit (iirc, it applies to omapdss and probably others), that it would be cleaner to expose the limit to userspace. So there should be at least a couple other drivers that could avoid virtualizing planes with some help from userspace for this case. Regarding rotation, I'm not 100% sure.. seems like we could just document these as the un-rotated limits. If we really had to, we could do some sort of dance where userspace sets rotation property on an un-used plane, and then reads-back the current values of the read-only prop's. But that seems awkward. BR, -R > If possible for your hw I'm heavily leaning towards this last approach. If > fits entirely into the current atomic design, and all the complexity is > restricted to your driver (you need to have some allocation map between > drm planes and real hw planes, but that's it). > > Thoughts? > -Daniel > >> > >> Archit >> >> > >> >Cheers, Daniel >> > >> >>Signed-off-by: Archit Taneja >> >>--- >> >> drivers/gpu/drm/drm_atomic.c | 9 + >> >> drivers/gpu/drm/drm_crtc.c | 38
[RFC] drm: Introduce max width and height properties for planes
High resoultion modes (4K/5K) are supported by encoder/crtc hardware, but
the backing hardware planes to scanout these buffers generally don't
support such large widths. Two (or more) hardware planes are used in
conjunction to scan out the entire width.
One way to support this is to virtualize the drm_plane objects that we
expose to userspace, and let the kms driver internally figure out whether
it needs to club multiple hardware pipes to run a particular mode.
The other option is to represent a drm_plane as it is in hardware, and
leave it to userspace use to manage multiple drm_planes for scanning
out these modes.
The advantage of the latter option is that the kms driver doesn't get
too complicated. It doesn't need to do the decision making on how to
distibute the hardware resources among the drm_planes. It also allows
userspace to continue doing some of the heavy lifting (figure out
the most power efficient combination of pipes, estimate bandwidth
consumption etc) which would have to be moved to the kernel if we
virtualized planes.
In order to achieve the latter, a drm_plane should expose an immutable
property to userspace describing the maximum width and height it can
support. Userspace can then prepare the drm_planes accordingly and
commit the state. This patch introduces these properties.
The main disadvantage is that legacy userspace (i.e. calling SetCrtc
on a high resolution framebuffer) wouldn't work. This, on the other
hand, wouldn't be a problem if we virtualized the planes and manipulated
the hardware pipes in the kenrel. One way around this is to fail large
resolution modes when using legacy setcrtc, or introduce some minimal
helpers in the kernel that automatically use multiple drm_planes when
we try to set such a mode. A solution to this issue isn't a part of the
RFC yet.
We're looking for feedback here and wondering whether this is the right
way to go or not.
Signed-off-by: Archit Taneja
---
drivers/gpu/drm/drm_atomic.c | 9 +
drivers/gpu/drm/drm_crtc.c | 38 ++
include/drm/drm_crtc.h | 6 ++
3 files changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8ee1db8..fded22a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -839,6 +839,15 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
return -ERANGE;
}
+ if (plane->max_width && (state->src_w >> 16) > plane->max_width ||
+ plane->max_height && (state->src_h >> 16) > plane->max_height) {
+ DRM_DEBUG_ATOMIC("Invalid source width/height "
+"%u.%06ux%u.%06u\n",
+state->src_w >> 16, ((state->src_w & 0x) *
15625) >> 10,
+state->src_h >> 16, ((state->src_h & 0x) *
15625) >> 10);
+ return -ERANGE;
+ }
+
fb_width = state->fb->width << 16;
fb_height = state->fb->height << 16;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e08f962..f2d3b92 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1267,6 +1267,9 @@ int drm_universal_plane_init(struct drm_device *dev,
struct drm_plane *plane,
plane->possible_crtcs = possible_crtcs;
plane->type = type;
+ plane->max_width = 0;
+ plane->max_height = 0;
+
list_add_tail(&plane->head, &config->plane_list);
config->num_total_plane++;
if (plane->type == DRM_PLANE_TYPE_OVERLAY)
@@ -4957,6 +4960,41 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
}
EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
+int drm_plane_create_max_width_prop(struct drm_plane *plane, uint32_t
max_width)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create_range(plane->dev, DRM_MODE_PROP_IMMUTABLE,
+"MAX_W", max_width, max_width);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, max_width);
+
+ plane->max_width = max_width;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_max_width_prop);
+
+int drm_plane_create_max_height_prop(struct drm_plane *plane,
+uint32_t max_height)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create_range(plane->dev, DRM_MODE_PROP_IMMUTABLE,
+"MAX_H", max_height, max_height);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, max_height);
+
+ plane->max_height = max_height;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_max_height_prop);
+
/**
* drm_mode_obj_get_properties_ioctl - get the current value of a object's
property
* @dev: DRM device
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e0170bf..6104527 100644
--- a/include/drm/drm_
[RFC] drm: Introduce max width and height properties for planes
On Wed, May 25, 2016 at 12:03:39PM +0530, Archit Taneja wrote:
> High resoultion modes (4K/5K) are supported by encoder/crtc hardware, but
> the backing hardware planes to scanout these buffers generally don't
> support such large widths. Two (or more) hardware planes are used in
> conjunction to scan out the entire width.
"generally" seems to be a bit an overstatement. Most sensible hw has no
problem at all scanning that kind of stuff out ;-)
> One way to support this is to virtualize the drm_plane objects that we
> expose to userspace, and let the kms driver internally figure out whether
> it needs to club multiple hardware pipes to run a particular mode.
>
> The other option is to represent a drm_plane as it is in hardware, and
> leave it to userspace use to manage multiple drm_planes for scanning
> out these modes.
>
> The advantage of the latter option is that the kms driver doesn't get
> too complicated. It doesn't need to do the decision making on how to
> distibute the hardware resources among the drm_planes. It also allows
> userspace to continue doing some of the heavy lifting (figure out
> the most power efficient combination of pipes, estimate bandwidth
> consumption etc) which would have to be moved to the kernel if we
> virtualized planes.
>
> In order to achieve the latter, a drm_plane should expose an immutable
> property to userspace describing the maximum width and height it can
> support. Userspace can then prepare the drm_planes accordingly and
> commit the state. This patch introduces these properties.
>
> The main disadvantage is that legacy userspace (i.e. calling SetCrtc
> on a high resolution framebuffer) wouldn't work. This, on the other
> hand, wouldn't be a problem if we virtualized the planes and manipulated
> the hardware pipes in the kenrel. One way around this is to fail large
> resolution modes when using legacy setcrtc, or introduce some minimal
> helpers in the kernel that automatically use multiple drm_planes when
> we try to set such a mode. A solution to this issue isn't a part of the
> RFC yet.
>
> We're looking for feedback here and wondering whether this is the right
> way to go or not.
So just this week there was a bit a discussion on irc (and I haven't
gotten around to typing the writeup and asking everyone for feedback on
it) on new properties. Super-short summary:
- new properties are new abi like anything else
- drm wants a userspace demonstration vehicle to prove new abi which is:
open source, patches reviewed & tested by respective canonical upstream
(so not a vendor fork or something like that). Testcases are explicitly
not enough, it needs to be the real deal.
- for props specifically which are meant to be used/interpreted by
compositor this means a patch for drm_hwcomposer, one of the many
wayland compositors there are (weston, ozone, mutter), or a patch for
xfree86-video-modesetting. Other userspace thingies are probably a bit
too much fringe to count.
- this isn't really new, but thus far arm simply sailed in the shadow of
intel doing all that hard work. Now some arm drivers start to pull
ahead, adding new ABI that's not yet proven by i915 folks, and with
success comes a bit more responsibility.
For the interface itself just a few questions:
- We should make the cursor size stuff obselete by this, and instead first
look up the size limits of the cursor plane, before checking those
legacy limits.
- Is the size/width really independent of e.g. rotation/pixel format/...
Should it be the maximum that's possible under best circumstance (things
could still fail), or the minimum that's guaranteed to work everwhere.
This is the kind of stuff we need the userspace part for, too.
Cheers, Daniel
> Signed-off-by: Archit Taneja
> ---
> drivers/gpu/drm/drm_atomic.c | 9 +
> drivers/gpu/drm/drm_crtc.c | 38 ++
> include/drm/drm_crtc.h | 6 ++
> 3 files changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 8ee1db8..fded22a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -839,6 +839,15 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ERANGE;
> }
>
> + if (plane->max_width && (state->src_w >> 16) > plane->max_width ||
> + plane->max_height && (state->src_h >> 16) > plane->max_height) {
> + DRM_DEBUG_ATOMIC("Invalid source width/height "
> + "%u.%06ux%u.%06u\n",
> + state->src_w >> 16, ((state->src_w & 0x) *
> 15625) >> 10,
> + state->src_h >> 16, ((state->src_h & 0x) *
> 15625) >> 10);
> + return -ERANGE;
> + }
> +
> fb_width = state->fb->width << 16;
> fb_height = state->fb->height << 16;
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e08f962..f
