Message ID | 20191017192601.GA215957@art_vandelay (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PULL] drm-misc-next | expand |
Hi Sean, On 17/10/2019 22:26, Sean Paul wrote: > concern for those. The omap OMAP_BO_MEM_* changes though I don't think have > really reached non-TI eyes. There's no link in the commit message to a UAPI > implementation and the only reference I could find is libkmsxx which can set > them through the python bindings. Since this is TI-specific gunk in TI-specific > headers I'm inclined to let it pass, but I would have liked to have this > conversation upfront. I figured I'd call this out so you have final say. There was some discussion about that a few years back when I initially sent the patches, but now that I look, the discussion died before really even starting. This is what I said about userspace implementation: > Yes, unfortunately that is not going to happen. I don't see how this > could be used in a generic system like Weston or X. It's meant for very > specific use cases, where you know exactly the requirements of your > application and the capabilities of the whole system, and optimize based > on that. I know this feature is used by customers, but I don't have access to their applications. Tomi
On Fri, Oct 18, 2019 at 9:46 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > Hi Sean, > > On 17/10/2019 22:26, Sean Paul wrote: > > > concern for those. The omap OMAP_BO_MEM_* changes though I don't think have > > really reached non-TI eyes. There's no link in the commit message to a UAPI > > implementation and the only reference I could find is libkmsxx which can set > > them through the python bindings. Since this is TI-specific gunk in TI-specific > > headers I'm inclined to let it pass, but I would have liked to have this > > conversation upfront. I figured I'd call this out so you have final say. > > There was some discussion about that a few years back when I initially > sent the patches, but now that I look, the discussion died before really > even starting. > > This is what I said about userspace implementation: > > > Yes, unfortunately that is not going to happen. I don't see how this > > could be used in a generic system like Weston or X. It's meant for very > > specific use cases, where you know exactly the requirements of your > > application and the capabilities of the whole system, and optimize based > > on that. Thanks for the context, Tomi. Indeed it looks like the discussion died, but Laurent still brought up the u/s requirement and the patch was effectively dead until Daniel or Dave weighed in. I'd expect at least some outreach before pushing the patch directly 2+ years later. Has anything changed since then? > I know this feature is used by customers, but I don't have access to > their applications. Unfortunately, and as you know, that is insufficient/irrelevant for introducing new UAPI. So the question is if the libkmsxx bindings constitute opensource userspace, it's really thin IMO. Sean > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi, On 18/10/2019 23:11, Sean Paul wrote: > On Fri, Oct 18, 2019 at 9:46 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >> >> Hi Sean, >> >> On 17/10/2019 22:26, Sean Paul wrote: >> >>> concern for those. The omap OMAP_BO_MEM_* changes though I don't think have >>> really reached non-TI eyes. There's no link in the commit message to a UAPI >>> implementation and the only reference I could find is libkmsxx which can set >>> them through the python bindings. Since this is TI-specific gunk in TI-specific >>> headers I'm inclined to let it pass, but I would have liked to have this >>> conversation upfront. I figured I'd call this out so you have final say. >> >> There was some discussion about that a few years back when I initially >> sent the patches, but now that I look, the discussion died before really >> even starting. >> >> This is what I said about userspace implementation: >> >>> Yes, unfortunately that is not going to happen. I don't see how this >>> could be used in a generic system like Weston or X. It's meant for very >>> specific use cases, where you know exactly the requirements of your >>> application and the capabilities of the whole system, and optimize based >>> on that. > > Thanks for the context, Tomi. > > Indeed it looks like the discussion died, but Laurent still brought up > the u/s requirement and the patch was effectively dead until Daniel or > Dave weighed in. I'd expect at least some outreach before pushing the > patch directly 2+ years later. Has anything changed since then? There were new review rounds for the series this summer & autumn, but no, nothing else. I haven't specifically pinged anyone about the UAPI changes. This series introduces three new flags to an already existing UAPI, and, for whatever reason, this didn't register to me as a new UAPI, even if Laurent asked about it some years back. So, my mistake. The flags are added in a single patch, so I can easily push a revert for that if this patch is not acceptable. The rest of the series is cleanup. >> I know this feature is used by customers, but I don't have access to >> their applications. > > Unfortunately, and as you know, that is insufficient/irrelevant for > introducing new UAPI. So the question is if the libkmsxx bindings > constitute opensource userspace, it's really thin IMO. Ok. Well, I know and understand the general rule here. But perhaps I haven't taken it as strictly as needed. I have to say I don't quite understand why this rule would be always strictly held, though. These flags, for example, what should we do with them? - They provide a proven, real use-case benefit. - They are specific to SoCs with OMAP DSS and DMM IPs. - They are relatively simple. - All the details, including the details about the HW, are public. - Using them makes sense only in cases where you are tuning your system to your application, and you must know the resource needs of all the apps in your system. So they cannot really be used in any generic setup, or at least I have no idea how. Does the history and experience say that such specific case as above cases don't really exist, and we can always have a generic UAPI (or, in worst case, a device specific UAPI) which can be used from X/Weston/or-such? Or do things like above exist, but they need to carried in vendors' kernels? Tomi
On Mon, Oct 21, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > Hi, > > On 18/10/2019 23:11, Sean Paul wrote: > > On Fri, Oct 18, 2019 at 9:46 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > >> > >> Hi Sean, > >> > >> On 17/10/2019 22:26, Sean Paul wrote: > >> > >>> concern for those. The omap OMAP_BO_MEM_* changes though I don't think have > >>> really reached non-TI eyes. There's no link in the commit message to a UAPI > >>> implementation and the only reference I could find is libkmsxx which can set > >>> them through the python bindings. Since this is TI-specific gunk in TI-specific > >>> headers I'm inclined to let it pass, but I would have liked to have this > >>> conversation upfront. I figured I'd call this out so you have final say. > >> > >> There was some discussion about that a few years back when I initially > >> sent the patches, but now that I look, the discussion died before really > >> even starting. > >> > >> This is what I said about userspace implementation: > >> > >>> Yes, unfortunately that is not going to happen. I don't see how this > >>> could be used in a generic system like Weston or X. It's meant for very > >>> specific use cases, where you know exactly the requirements of your > >>> application and the capabilities of the whole system, and optimize based > >>> on that. > > > > Thanks for the context, Tomi. > > > > Indeed it looks like the discussion died, but Laurent still brought up > > the u/s requirement and the patch was effectively dead until Daniel or > > Dave weighed in. I'd expect at least some outreach before pushing the > > patch directly 2+ years later. Has anything changed since then? > > There were new review rounds for the series this summer & autumn, but > no, nothing else. I haven't specifically pinged anyone about the UAPI > changes. > > This series introduces three new flags to an already existing UAPI, and, > for whatever reason, this didn't register to me as a new UAPI, even if > Laurent asked about it some years back. > > So, my mistake. > > The flags are added in a single patch, so I can easily push a revert for > that if this patch is not acceptable. The rest of the series is cleanup. > I'll wait for Dave or Daniel to weigh in on whether they want to take this, otherwise I'll revert before sending the next pull and we can have this conversation on the review. > >> I know this feature is used by customers, but I don't have access to > >> their applications. > > > > Unfortunately, and as you know, that is insufficient/irrelevant for > > introducing new UAPI. So the question is if the libkmsxx bindings > > constitute opensource userspace, it's really thin IMO. > > Ok. Well, I know and understand the general rule here. But perhaps I > haven't taken it as strictly as needed. > > I have to say I don't quite understand why this rule would be always > strictly held, though. > It's strictly held because once you start accepting the harmless/isolated features every driver has, you end up with untestable bloat sprinkled everywhere. > These flags, for example, what should we do with them? > - They provide a proven, real use-case benefit. > - They are specific to SoCs with OMAP DSS and DMM IPs. > - They are relatively simple. > - All the details, including the details about the HW, are public. > - Using them makes sense only in cases where you are tuning your system > to your application, and you must know the resource needs of all the > apps in your system. So they cannot really be used in any generic setup, > or at least I have no idea how. > > Does the history and experience say that such specific case as above > cases don't really exist, and we can always have a generic UAPI (or, in > worst case, a device specific UAPI) which can be used from X/Weston/or-such? > We don't need generic userspace to be able to make use of it, but at least some oss project should find it useful. Otherwise why are we maintaining code that no one in the open source community cares about? How do we test it when the closed source implementations have been abandoned? > Or do things like above exist, but they need to carried in vendors' kernels? That's really the problem. _Everybody_ has features they would describe as above, so where do we draw the line? Sean > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 22 Oct 2019 at 01:49, Sean Paul <sean@poorly.run> wrote: > > On Mon, Oct 21, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > > > Hi, > > > > On 18/10/2019 23:11, Sean Paul wrote: > > > On Fri, Oct 18, 2019 at 9:46 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > >> > > >> Hi Sean, > > >> > > >> On 17/10/2019 22:26, Sean Paul wrote: > > >> > > >>> concern for those. The omap OMAP_BO_MEM_* changes though I don't think have > > >>> really reached non-TI eyes. There's no link in the commit message to a UAPI > > >>> implementation and the only reference I could find is libkmsxx which can set > > >>> them through the python bindings. Since this is TI-specific gunk in TI-specific > > >>> headers I'm inclined to let it pass, but I would have liked to have this > > >>> conversation upfront. I figured I'd call this out so you have final say. > > >> > > >> There was some discussion about that a few years back when I initially > > >> sent the patches, but now that I look, the discussion died before really > > >> even starting. > > >> > > >> This is what I said about userspace implementation: > > >> > > >>> Yes, unfortunately that is not going to happen. I don't see how this > > >>> could be used in a generic system like Weston or X. It's meant for very > > >>> specific use cases, where you know exactly the requirements of your > > >>> application and the capabilities of the whole system, and optimize based > > >>> on that. > > > > > > Thanks for the context, Tomi. > > > > > > Indeed it looks like the discussion died, but Laurent still brought up > > > the u/s requirement and the patch was effectively dead until Daniel or > > > Dave weighed in. I'd expect at least some outreach before pushing the > > > patch directly 2+ years later. Has anything changed since then? > > > > There were new review rounds for the series this summer & autumn, but > > no, nothing else. I haven't specifically pinged anyone about the UAPI > > changes. > > > > This series introduces three new flags to an already existing UAPI, and, > > for whatever reason, this didn't register to me as a new UAPI, even if > > Laurent asked about it some years back. > > > > So, my mistake. > > > > The flags are added in a single patch, so I can easily push a revert for > > that if this patch is not acceptable. The rest of the series is cleanup. > > > > I'll wait for Dave or Daniel to weigh in on whether they want to take > this, otherwise I'll revert before sending the next pull and we can > have this conversation on the review. I'd rather we revert it, just to stick to some semblance of the rules, otherwise if we make execptions other people will drive trucks through them. Dave.
On Tue, Oct 22, 2019 at 4:17 AM Dave Airlie <airlied@gmail.com> wrote: > > On Tue, 22 Oct 2019 at 01:49, Sean Paul <sean@poorly.run> wrote: > > > > On Mon, Oct 21, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > > > > > Hi, > > > > > > On 18/10/2019 23:11, Sean Paul wrote: > > > > On Fri, Oct 18, 2019 at 9:46 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > > > >> > > > >> Hi Sean, > > > >> > > > >> On 17/10/2019 22:26, Sean Paul wrote: > > > >> > > > >>> concern for those. The omap OMAP_BO_MEM_* changes though I don't think have > > > >>> really reached non-TI eyes. There's no link in the commit message to a UAPI > > > >>> implementation and the only reference I could find is libkmsxx which can set > > > >>> them through the python bindings. Since this is TI-specific gunk in TI-specific > > > >>> headers I'm inclined to let it pass, but I would have liked to have this > > > >>> conversation upfront. I figured I'd call this out so you have final say. > > > >> > > > >> There was some discussion about that a few years back when I initially > > > >> sent the patches, but now that I look, the discussion died before really > > > >> even starting. > > > >> > > > >> This is what I said about userspace implementation: > > > >> > > > >>> Yes, unfortunately that is not going to happen. I don't see how this > > > >>> could be used in a generic system like Weston or X. It's meant for very > > > >>> specific use cases, where you know exactly the requirements of your > > > >>> application and the capabilities of the whole system, and optimize based > > > >>> on that. > > > > > > > > Thanks for the context, Tomi. > > > > > > > > Indeed it looks like the discussion died, but Laurent still brought up > > > > the u/s requirement and the patch was effectively dead until Daniel or > > > > Dave weighed in. I'd expect at least some outreach before pushing the > > > > patch directly 2+ years later. Has anything changed since then? > > > > > > There were new review rounds for the series this summer & autumn, but > > > no, nothing else. I haven't specifically pinged anyone about the UAPI > > > changes. > > > > > > This series introduces three new flags to an already existing UAPI, and, > > > for whatever reason, this didn't register to me as a new UAPI, even if > > > Laurent asked about it some years back. > > > > > > So, my mistake. > > > > > > The flags are added in a single patch, so I can easily push a revert for > > > that if this patch is not acceptable. The rest of the series is cleanup. > > > > > > > I'll wait for Dave or Daniel to weigh in on whether they want to take > > this, otherwise I'll revert before sending the next pull and we can > > have this conversation on the review. > > I'd rather we revert it, just to stick to some semblance of the rules, > otherwise if we make execptions other people will drive trucks through > them. Imo we have driven a truck through the "it's not really new uapi, only a new flag/field/type added to existing uapi" + "it's obvious this is the right thing", it's how we got a metric ton of questionable kms properties. Also for this case specifically, I'm not seeing why we can't follow the usual rules: - lots of gem drivers have buffer allocation/placement hints (it's kinda the thing ttm is for ...), and they all found some userspace for it - _PIN looks suspicious, imo the proper approach is something like amdgpu's per-ctx persistent working set, which gives you the "look no pin" fastpath while still being able to manage buffers for real. That one also yells at you with ENOMEM if the memory doesn't exist, at alloc time. tldr; I concur Cheers, Daniel