diff mbox

[libdrm] headers: Add README file

Message ID 20161110164423.588-1-emil.l.velikov@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Emil Velikov Nov. 10, 2016, 4:44 p.m. UTC
From: Emil Velikov <emil.velikov@collabora.com>

Since we're trying to standardise and make things more consistent in
the area, add a basic README which covers some of the more popular
topics.

Cc: Dave Airlie <airlied@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
Dave, did I get it right on the "why only drm files should live here" ?

Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
point here ?
---
 include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 include/drm/README

Comments

Daniel Vetter Nov. 10, 2016, 8:01 p.m. UTC | #1
On Thu, Nov 10, 2016 at 04:44:23PM +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> Since we're trying to standardise and make things more consistent in
> the area, add a basic README which covers some of the more popular
> topics.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> Dave, did I get it right on the "why only drm files should live here" ?
> 
> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
> point here ?

Not sure we need drm-misc really, I tend to send regular pull requests to
Dave anyway. But in principle any -next branch is good enough, as long as
it contains all the previous uapi updates already (to avoid regressions).
And git repos for this stuff can be found in the kernel's MAINTAINERS
file. Imo your current wording is good enough.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 include/drm/README
> 
> diff --git a/include/drm/README b/include/drm/README
> new file mode 100644
> index 0000000..2f80c15
> --- /dev/null
> +++ b/include/drm/README
> @@ -0,0 +1,72 @@
> +What are these headers ?
> +------------------------
> +This is the canonical source of drm headers that user space should use for
> +communicating with the kernel DRM subsystem.
> +
> +They flow from the kernel, thus any changes must be merged there first.
> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
> +
> +
> +Non-linux platforms - changes/patches
> +-------------------------------------
> +If your platform has local changes, please send them upstream for inclusion.
> +Even if your patches don't get accepted in their current form, devs will
> +give you feedback on how to address things properly.
> +
> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
> +mailing list.
> +
> +Before doing so, please consider the following:
> + - Have the [libdrm vs kernel] headers on your platform deviated ?
> +Consider unifying them first.
> +
> + - Have you introduced additional ABI that's not available in Linux ?
> +Propose it for [Linux kernel] upstream inclusion.
> +If that doesn't work out (hopefully it never does), move it to another header
> +and/or keep the change(s) local ?
> +
> + - Are your changes DRI1/UMS specific ?
> +There is virtually no interest/power in keeping those legacy interfaces. They
> +are around due to the kernel "thou shalt not break existing user space" rule.
> +
> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
> +capable of supporting those.
> +
> +
> +Which headers go where ?
> +------------------------
> +A snipped from the, now removed, Makefile.am used to state:
> +
> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
> +  however, r300 and via need their reg headers installed in order to build.
> +  better solutions are welcome.
> +
> +Obviously the r300 and via headers are no longer around ;-)
> +
> +Reason behind is that the drm headers can be used as a basic communications
> +channel with the respective kernel modules. If more advanced functionality is
> +required one can pull the specific libdrm_$driver which is free to pull
> +additional files from the kernel.
> +
> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
> +
> +If your driver is still in prototyping/staging state, consider moving the
> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
> +definitions and access [via $driver_drmif.h or similar] would be better fit.
> +
> +
> +When and how to update these files
> +----------------------------------
> +Ideally on each libdrm release these will be kept in sync, with the latest
> +released kernels. This way users won't need to provide local definitions.
> +
> +In order to update the files do the following:
> + - Switch to a Linux kernel tree/branch which is not rebased.
> +For example: airlied/drm-next, drm-misc/XXX
> + - Install the headers via `make headers_install' to a separate location.
> + - Copy the drm header[s] + git add + git commit.
> + - Note: Your commit message must include:
> +   a) Brief summary on the delta. If there's any change that looks like an
> +API/ABI break one _must_ explicitly state why it's safe to do so.
> +   b) "Generated using make headers_install."
> +   c) "Generated from $tree/branch commit $sha"
> -- 
> 2.9.3
>
Alex Deucher Nov. 10, 2016, 9:07 p.m. UTC | #2
On Thu, Nov 10, 2016 at 11:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Since we're trying to standardise and make things more consistent in
> the area, add a basic README which covers some of the more popular
> topics.
>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> Dave, did I get it right on the "why only drm files should live here" ?
>
> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
> point here ?
> ---
>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 include/drm/README
>
> diff --git a/include/drm/README b/include/drm/README
> new file mode 100644
> index 0000000..2f80c15
> --- /dev/null
> +++ b/include/drm/README
> @@ -0,0 +1,72 @@
> +What are these headers ?
> +------------------------
> +This is the canonical source of drm headers that user space should use for
> +communicating with the kernel DRM subsystem.
> +
> +They flow from the kernel, thus any changes must be merged there first.
> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
> +
> +
> +Non-linux platforms - changes/patches
> +-------------------------------------
> +If your platform has local changes, please send them upstream for inclusion.
> +Even if your patches don't get accepted in their current form, devs will
> +give you feedback on how to address things properly.
> +
> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
> +mailing list.
> +
> +Before doing so, please consider the following:
> + - Have the [libdrm vs kernel] headers on your platform deviated ?
> +Consider unifying them first.
> +
> + - Have you introduced additional ABI that's not available in Linux ?
> +Propose it for [Linux kernel] upstream inclusion.
> +If that doesn't work out (hopefully it never does), move it to another header
> +and/or keep the change(s) local ?
> +
> + - Are your changes DRI1/UMS specific ?
> +There is virtually no interest/power in keeping those legacy interfaces. They
> +are around due to the kernel "thou shalt not break existing user space" rule.
> +
> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
> +capable of supporting those.
> +
> +
> +Which headers go where ?
> +------------------------
> +A snipped from the, now removed, Makefile.am used to state:
> +
> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
> +  however, r300 and via need their reg headers installed in order to build.
> +  better solutions are welcome.
> +
> +Obviously the r300 and via headers are no longer around ;-)
> +
> +Reason behind is that the drm headers can be used as a basic communications
> +channel with the respective kernel modules. If more advanced functionality is
> +required one can pull the specific libdrm_$driver which is free to pull
> +additional files from the kernel.
> +
> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
> +
> +If your driver is still in prototyping/staging state, consider moving the
> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
> +definitions and access [via $driver_drmif.h or similar] would be better fit.
> +
> +
> +When and how to update these files
> +----------------------------------
> +Ideally on each libdrm release these will be kept in sync, with the latest
> +released kernels. This way users won't need to provide local definitions.
> +
> +In order to update the files do the following:
> + - Switch to a Linux kernel tree/branch which is not rebased.
> +For example: airlied/drm-next, drm-misc/XXX

If we just want to update it to the latest released kernel, why not
just specify Linus' tree?  There's a chance there may be flux in -next
that you wouldn't necessarily want in libdrm.  Also, I think
generally, it would be the individual driver maintainers or people
working on specific core features that do this.  Does it really make
sense to update these en masse regularly?

Alex

> + - Install the headers via `make headers_install' to a separate location.
> + - Copy the drm header[s] + git add + git commit.
> + - Note: Your commit message must include:
> +   a) Brief summary on the delta. If there's any change that looks like an
> +API/ABI break one _must_ explicitly state why it's safe to do so.
> +   b) "Generated using make headers_install."
> +   c) "Generated from $tree/branch commit $sha"
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Emil Velikov Nov. 11, 2016, 1:44 p.m. UTC | #3
On 10 November 2016 at 21:07, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Nov 10, 2016 at 11:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> From: Emil Velikov <emil.velikov@collabora.com>
>>
>> Since we're trying to standardise and make things more consistent in
>> the area, add a basic README which covers some of the more popular
>> topics.
>>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>> ---
>> Dave, did I get it right on the "why only drm files should live here" ?
>>
>> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
>> point here ?
>> ---
>>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 72 insertions(+)
>>  create mode 100644 include/drm/README
>>
>> diff --git a/include/drm/README b/include/drm/README
>> new file mode 100644
>> index 0000000..2f80c15
>> --- /dev/null
>> +++ b/include/drm/README
>> @@ -0,0 +1,72 @@
>> +What are these headers ?
>> +------------------------
>> +This is the canonical source of drm headers that user space should use for
>> +communicating with the kernel DRM subsystem.
>> +
>> +They flow from the kernel, thus any changes must be merged there first.
>> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
>> +
>> +
>> +Non-linux platforms - changes/patches
>> +-------------------------------------
>> +If your platform has local changes, please send them upstream for inclusion.
>> +Even if your patches don't get accepted in their current form, devs will
>> +give you feedback on how to address things properly.
>> +
>> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
>> +mailing list.
>> +
>> +Before doing so, please consider the following:
>> + - Have the [libdrm vs kernel] headers on your platform deviated ?
>> +Consider unifying them first.
>> +
>> + - Have you introduced additional ABI that's not available in Linux ?
>> +Propose it for [Linux kernel] upstream inclusion.
>> +If that doesn't work out (hopefully it never does), move it to another header
>> +and/or keep the change(s) local ?
>> +
>> + - Are your changes DRI1/UMS specific ?
>> +There is virtually no interest/power in keeping those legacy interfaces. They
>> +are around due to the kernel "thou shalt not break existing user space" rule.
>> +
>> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
>> +capable of supporting those.
>> +
>> +
>> +Which headers go where ?
>> +------------------------
>> +A snipped from the, now removed, Makefile.am used to state:
>> +
>> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
>> +  however, r300 and via need their reg headers installed in order to build.
>> +  better solutions are welcome.
>> +
>> +Obviously the r300 and via headers are no longer around ;-)
>> +
>> +Reason behind is that the drm headers can be used as a basic communications
>> +channel with the respective kernel modules. If more advanced functionality is
>> +required one can pull the specific libdrm_$driver which is free to pull
>> +additional files from the kernel.
>> +
>> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
>> +
>> +If your driver is still in prototyping/staging state, consider moving the
>> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
>> +definitions and access [via $driver_drmif.h or similar] would be better fit.
>> +
>> +
>> +When and how to update these files
>> +----------------------------------
>> +Ideally on each libdrm release these will be kept in sync, with the latest
>> +released kernels. This way users won't need to provide local definitions.
>> +
>> +In order to update the files do the following:
>> + - Switch to a Linux kernel tree/branch which is not rebased.
>> +For example: airlied/drm-next, drm-misc/XXX
>
> If we just want to update it to the latest released kernel, why not
> just specify Linus' tree?  There's a chance there may be flux in -next
> that you wouldn't necessarily want in libdrm.
My understanding is that things are "fully carved in stone" only as
they reach Linus. Yet things in drm-next are good enough.

>  Also, I think
> generally, it would be the individual driver maintainers or people
> working on specific core features that do this.  Does it really make
> sense to update these en masse regularly?
>
Ideally we'll mass import (update only) from Linus and do individuals
(from -next) as devs. deem fit. We want the former since devs can
forget about the latter.
Former is "not there yet", so I'll add a mention on the whole topic.

Speaking of which - can anyone from the team skim through amdgpu_drm.h
and radeon_drm.h update them.
Former is trivial, while the latter needs a closer look:
 - missing (trailing) padding -
drm_radeon_gem_{create,{g,s}et_tiling,set_domain} others ?
 - "broken" API - missing RADEON_TILING_R600_NO_SCANOUT, CIK_TILE_MODE_*

Thanks
Emil
Alex Deucher Nov. 11, 2016, 4:21 p.m. UTC | #4
On Fri, Nov 11, 2016 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 10 November 2016 at 21:07, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Thu, Nov 10, 2016 at 11:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> From: Emil Velikov <emil.velikov@collabora.com>
>>>
>>> Since we're trying to standardise and make things more consistent in
>>> the area, add a basic README which covers some of the more popular
>>> topics.
>>>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>>> ---
>>> Dave, did I get it right on the "why only drm files should live here" ?
>>>
>>> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
>>> point here ?
>>> ---
>>>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>  create mode 100644 include/drm/README
>>>
>>> diff --git a/include/drm/README b/include/drm/README
>>> new file mode 100644
>>> index 0000000..2f80c15
>>> --- /dev/null
>>> +++ b/include/drm/README
>>> @@ -0,0 +1,72 @@
>>> +What are these headers ?
>>> +------------------------
>>> +This is the canonical source of drm headers that user space should use for
>>> +communicating with the kernel DRM subsystem.
>>> +
>>> +They flow from the kernel, thus any changes must be merged there first.
>>> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
>>> +
>>> +
>>> +Non-linux platforms - changes/patches
>>> +-------------------------------------
>>> +If your platform has local changes, please send them upstream for inclusion.
>>> +Even if your patches don't get accepted in their current form, devs will
>>> +give you feedback on how to address things properly.
>>> +
>>> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
>>> +mailing list.
>>> +
>>> +Before doing so, please consider the following:
>>> + - Have the [libdrm vs kernel] headers on your platform deviated ?
>>> +Consider unifying them first.
>>> +
>>> + - Have you introduced additional ABI that's not available in Linux ?
>>> +Propose it for [Linux kernel] upstream inclusion.
>>> +If that doesn't work out (hopefully it never does), move it to another header
>>> +and/or keep the change(s) local ?
>>> +
>>> + - Are your changes DRI1/UMS specific ?
>>> +There is virtually no interest/power in keeping those legacy interfaces. They
>>> +are around due to the kernel "thou shalt not break existing user space" rule.
>>> +
>>> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
>>> +capable of supporting those.
>>> +
>>> +
>>> +Which headers go where ?
>>> +------------------------
>>> +A snipped from the, now removed, Makefile.am used to state:
>>> +
>>> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
>>> +  however, r300 and via need their reg headers installed in order to build.
>>> +  better solutions are welcome.
>>> +
>>> +Obviously the r300 and via headers are no longer around ;-)
>>> +
>>> +Reason behind is that the drm headers can be used as a basic communications
>>> +channel with the respective kernel modules. If more advanced functionality is
>>> +required one can pull the specific libdrm_$driver which is free to pull
>>> +additional files from the kernel.
>>> +
>>> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
>>> +
>>> +If your driver is still in prototyping/staging state, consider moving the
>>> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
>>> +definitions and access [via $driver_drmif.h or similar] would be better fit.
>>> +
>>> +
>>> +When and how to update these files
>>> +----------------------------------
>>> +Ideally on each libdrm release these will be kept in sync, with the latest
>>> +released kernels. This way users won't need to provide local definitions.
>>> +
>>> +In order to update the files do the following:
>>> + - Switch to a Linux kernel tree/branch which is not rebased.
>>> +For example: airlied/drm-next, drm-misc/XXX
>>
>> If we just want to update it to the latest released kernel, why not
>> just specify Linus' tree?  There's a chance there may be flux in -next
>> that you wouldn't necessarily want in libdrm.
> My understanding is that things are "fully carved in stone" only as
> they reach Linus. Yet things in drm-next are good enough.
>
>>  Also, I think
>> generally, it would be the individual driver maintainers or people
>> working on specific core features that do this.  Does it really make
>> sense to update these en masse regularly?
>>
> Ideally we'll mass import (update only) from Linus and do individuals
> (from -next) as devs. deem fit. We want the former since devs can
> forget about the latter.
> Former is "not there yet", so I'll add a mention on the whole topic.
>
> Speaking of which - can anyone from the team skim through amdgpu_drm.h
> and radeon_drm.h update them.
> Former is trivial, while the latter needs a closer look:
>  - missing (trailing) padding -
> drm_radeon_gem_{create,{g,s}et_tiling,set_domain} others ?
>  - "broken" API - missing RADEON_TILING_R600_NO_SCANOUT, CIK_TILE_MODE_*

I think the extra stuff in radeon_drm.h in libdrm can go.  I think
Marek addressed this previously.  It was just stuck in there for
convenience IIRC.

Alex
Marek Olšák Nov. 11, 2016, 6:33 p.m. UTC | #5
On Fri, Nov 11, 2016 at 5:21 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Nov 11, 2016 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 10 November 2016 at 21:07, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Thu, Nov 10, 2016 at 11:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>>> From: Emil Velikov <emil.velikov@collabora.com>
>>>>
>>>> Since we're trying to standardise and make things more consistent in
>>>> the area, add a basic README which covers some of the more popular
>>>> topics.
>>>>
>>>> Cc: Dave Airlie <airlied@redhat.com>
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>>>> ---
>>>> Dave, did I get it right on the "why only drm files should live here" ?
>>>>
>>>> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
>>>> point here ?
>>>> ---
>>>>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 72 insertions(+)
>>>>  create mode 100644 include/drm/README
>>>>
>>>> diff --git a/include/drm/README b/include/drm/README
>>>> new file mode 100644
>>>> index 0000000..2f80c15
>>>> --- /dev/null
>>>> +++ b/include/drm/README
>>>> @@ -0,0 +1,72 @@
>>>> +What are these headers ?
>>>> +------------------------
>>>> +This is the canonical source of drm headers that user space should use for
>>>> +communicating with the kernel DRM subsystem.
>>>> +
>>>> +They flow from the kernel, thus any changes must be merged there first.
>>>> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
>>>> +
>>>> +
>>>> +Non-linux platforms - changes/patches
>>>> +-------------------------------------
>>>> +If your platform has local changes, please send them upstream for inclusion.
>>>> +Even if your patches don't get accepted in their current form, devs will
>>>> +give you feedback on how to address things properly.
>>>> +
>>>> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
>>>> +mailing list.
>>>> +
>>>> +Before doing so, please consider the following:
>>>> + - Have the [libdrm vs kernel] headers on your platform deviated ?
>>>> +Consider unifying them first.
>>>> +
>>>> + - Have you introduced additional ABI that's not available in Linux ?
>>>> +Propose it for [Linux kernel] upstream inclusion.
>>>> +If that doesn't work out (hopefully it never does), move it to another header
>>>> +and/or keep the change(s) local ?
>>>> +
>>>> + - Are your changes DRI1/UMS specific ?
>>>> +There is virtually no interest/power in keeping those legacy interfaces. They
>>>> +are around due to the kernel "thou shalt not break existing user space" rule.
>>>> +
>>>> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
>>>> +capable of supporting those.
>>>> +
>>>> +
>>>> +Which headers go where ?
>>>> +------------------------
>>>> +A snipped from the, now removed, Makefile.am used to state:
>>>> +
>>>> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
>>>> +  however, r300 and via need their reg headers installed in order to build.
>>>> +  better solutions are welcome.
>>>> +
>>>> +Obviously the r300 and via headers are no longer around ;-)
>>>> +
>>>> +Reason behind is that the drm headers can be used as a basic communications
>>>> +channel with the respective kernel modules. If more advanced functionality is
>>>> +required one can pull the specific libdrm_$driver which is free to pull
>>>> +additional files from the kernel.
>>>> +
>>>> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
>>>> +
>>>> +If your driver is still in prototyping/staging state, consider moving the
>>>> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
>>>> +definitions and access [via $driver_drmif.h or similar] would be better fit.
>>>> +
>>>> +
>>>> +When and how to update these files
>>>> +----------------------------------
>>>> +Ideally on each libdrm release these will be kept in sync, with the latest
>>>> +released kernels. This way users won't need to provide local definitions.
>>>> +
>>>> +In order to update the files do the following:
>>>> + - Switch to a Linux kernel tree/branch which is not rebased.
>>>> +For example: airlied/drm-next, drm-misc/XXX
>>>
>>> If we just want to update it to the latest released kernel, why not
>>> just specify Linus' tree?  There's a chance there may be flux in -next
>>> that you wouldn't necessarily want in libdrm.
>> My understanding is that things are "fully carved in stone" only as
>> they reach Linus. Yet things in drm-next are good enough.
>>
>>>  Also, I think
>>> generally, it would be the individual driver maintainers or people
>>> working on specific core features that do this.  Does it really make
>>> sense to update these en masse regularly?
>>>
>> Ideally we'll mass import (update only) from Linus and do individuals
>> (from -next) as devs. deem fit. We want the former since devs can
>> forget about the latter.
>> Former is "not there yet", so I'll add a mention on the whole topic.
>>
>> Speaking of which - can anyone from the team skim through amdgpu_drm.h
>> and radeon_drm.h update them.
>> Former is trivial, while the latter needs a closer look:
>>  - missing (trailing) padding -
>> drm_radeon_gem_{create,{g,s}et_tiling,set_domain} others ?
>>  - "broken" API - missing RADEON_TILING_R600_NO_SCANOUT, CIK_TILE_MODE_*
>
> I think the extra stuff in radeon_drm.h in libdrm can go.  I think
> Marek addressed this previously.  It was just stuck in there for
> convenience IIRC.

RADEON_TILING_R600_NO_SCANOUT must stay.

The rest can be moved to radeon/radeon_surface.c.

Compile-test Mesa and DDXs to verify that they are not broken by
changes in radeon_drm.h.

Marek
Emil Velikov Nov. 11, 2016, 7:22 p.m. UTC | #6
On 11 November 2016 at 18:33, Marek Olšák <maraeo@gmail.com> wrote:
> On Fri, Nov 11, 2016 at 5:21 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Fri, Nov 11, 2016 at 8:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> On 10 November 2016 at 21:07, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>> On Thu, Nov 10, 2016 at 11:44 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>>>> From: Emil Velikov <emil.velikov@collabora.com>
>>>>>
>>>>> Since we're trying to standardise and make things more consistent in
>>>>> the area, add a basic README which covers some of the more popular
>>>>> topics.
>>>>>
>>>>> Cc: Dave Airlie <airlied@redhat.com>
>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>>>>> ---
>>>>> Dave, did I get it right on the "why only drm files should live here" ?
>>>>>
>>>>> Dave, Daniel, which trees/branches [in drm-misc] we can use as reference
>>>>> point here ?
>>>>> ---
>>>>>  include/drm/README | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 72 insertions(+)
>>>>>  create mode 100644 include/drm/README
>>>>>
>>>>> diff --git a/include/drm/README b/include/drm/README
>>>>> new file mode 100644
>>>>> index 0000000..2f80c15
>>>>> --- /dev/null
>>>>> +++ b/include/drm/README
>>>>> @@ -0,0 +1,72 @@
>>>>> +What are these headers ?
>>>>> +------------------------
>>>>> +This is the canonical source of drm headers that user space should use for
>>>>> +communicating with the kernel DRM subsystem.
>>>>> +
>>>>> +They flow from the kernel, thus any changes must be merged there first.
>>>>> +Do _not_ attempt to "fix" these by deviating from the kernel ones !
>>>>> +
>>>>> +
>>>>> +Non-linux platforms - changes/patches
>>>>> +-------------------------------------
>>>>> +If your platform has local changes, please send them upstream for inclusion.
>>>>> +Even if your patches don't get accepted in their current form, devs will
>>>>> +give you feedback on how to address things properly.
>>>>> +
>>>>> +git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
>>>>> +mailing list.
>>>>> +
>>>>> +Before doing so, please consider the following:
>>>>> + - Have the [libdrm vs kernel] headers on your platform deviated ?
>>>>> +Consider unifying them first.
>>>>> +
>>>>> + - Have you introduced additional ABI that's not available in Linux ?
>>>>> +Propose it for [Linux kernel] upstream inclusion.
>>>>> +If that doesn't work out (hopefully it never does), move it to another header
>>>>> +and/or keep the change(s) local ?
>>>>> +
>>>>> + - Are your changes DRI1/UMS specific ?
>>>>> +There is virtually no interest/power in keeping those legacy interfaces. They
>>>>> +are around due to the kernel "thou shalt not break existing user space" rule.
>>>>> +
>>>>> +Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
>>>>> +capable of supporting those.
>>>>> +
>>>>> +
>>>>> +Which headers go where ?
>>>>> +------------------------
>>>>> +A snipped from the, now removed, Makefile.am used to state:
>>>>> +
>>>>> +  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
>>>>> +  however, r300 and via need their reg headers installed in order to build.
>>>>> +  better solutions are welcome.
>>>>> +
>>>>> +Obviously the r300 and via headers are no longer around ;-)
>>>>> +
>>>>> +Reason behind is that the drm headers can be used as a basic communications
>>>>> +channel with the respective kernel modules. If more advanced functionality is
>>>>> +required one can pull the specific libdrm_$driver which is free to pull
>>>>> +additional files from the kernel.
>>>>> +
>>>>> +For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
>>>>> +
>>>>> +If your driver is still in prototyping/staging state, consider moving the
>>>>> +$driver_drm.h into $driver and _not_ installing it. An header providing opaque
>>>>> +definitions and access [via $driver_drmif.h or similar] would be better fit.
>>>>> +
>>>>> +
>>>>> +When and how to update these files
>>>>> +----------------------------------
>>>>> +Ideally on each libdrm release these will be kept in sync, with the latest
>>>>> +released kernels. This way users won't need to provide local definitions.
>>>>> +
>>>>> +In order to update the files do the following:
>>>>> + - Switch to a Linux kernel tree/branch which is not rebased.
>>>>> +For example: airlied/drm-next, drm-misc/XXX
>>>>
>>>> If we just want to update it to the latest released kernel, why not
>>>> just specify Linus' tree?  There's a chance there may be flux in -next
>>>> that you wouldn't necessarily want in libdrm.
>>> My understanding is that things are "fully carved in stone" only as
>>> they reach Linus. Yet things in drm-next are good enough.
>>>
>>>>  Also, I think
>>>> generally, it would be the individual driver maintainers or people
>>>> working on specific core features that do this.  Does it really make
>>>> sense to update these en masse regularly?
>>>>
>>> Ideally we'll mass import (update only) from Linus and do individuals
>>> (from -next) as devs. deem fit. We want the former since devs can
>>> forget about the latter.
>>> Former is "not there yet", so I'll add a mention on the whole topic.
>>>
>>> Speaking of which - can anyone from the team skim through amdgpu_drm.h
>>> and radeon_drm.h update them.
>>> Former is trivial, while the latter needs a closer look:
>>>  - missing (trailing) padding -
>>> drm_radeon_gem_{create,{g,s}et_tiling,set_domain} others ?
>>>  - "broken" API - missing RADEON_TILING_R600_NO_SCANOUT, CIK_TILE_MODE_*
>>
>> I think the extra stuff in radeon_drm.h in libdrm can go.  I think
>> Marek addressed this previously.  It was just stuck in there for
>> convenience IIRC.
>
> RADEON_TILING_R600_NO_SCANOUT must stay.
>
> The rest can be moved to radeon/radeon_surface.c.
>
> Compile-test Mesa and DDXs to verify that they are not broken by
> changes in radeon_drm.h.
>
As much as I want to trying to fix everything - it isn't humanly
possible. I was hoping that one of you guys (colleague?) can give it a
spin.
Plus I'm bad at Chinese whispers [1]

Thanks
Emil
[1] https://en.wikipedia.org/wiki/Chinese_whispers
diff mbox

Patch

diff --git a/include/drm/README b/include/drm/README
new file mode 100644
index 0000000..2f80c15
--- /dev/null
+++ b/include/drm/README
@@ -0,0 +1,72 @@ 
+What are these headers ?
+------------------------
+This is the canonical source of drm headers that user space should use for
+communicating with the kernel DRM subsystem.
+
+They flow from the kernel, thus any changes must be merged there first.
+Do _not_ attempt to "fix" these by deviating from the kernel ones !
+
+
+Non-linux platforms - changes/patches
+-------------------------------------
+If your platform has local changes, please send them upstream for inclusion.
+Even if your patches don't get accepted in their current form, devs will
+give you feedback on how to address things properly.
+
+git send-email --subject-prefix="PATCH libdrm" your patches to dri-devel
+mailing list.
+
+Before doing so, please consider the following:
+ - Have the [libdrm vs kernel] headers on your platform deviated ?
+Consider unifying them first.
+
+ - Have you introduced additional ABI that's not available in Linux ?
+Propose it for [Linux kernel] upstream inclusion.
+If that doesn't work out (hopefully it never does), move it to another header
+and/or keep the change(s) local ?
+
+ - Are your changes DRI1/UMS specific ?
+There is virtually no interest/power in keeping those legacy interfaces. They
+are around due to the kernel "thou shalt not break existing user space" rule.
+
+Consider porting the driver to DRI2/KMS - all (almost?) sensible hardware is
+capable of supporting those.
+
+
+Which headers go where ?
+------------------------
+A snipped from the, now removed, Makefile.am used to state:
+
+  XXX airlied says, nothing besides *_drm.h and drm*.h should be necessary.
+  however, r300 and via need their reg headers installed in order to build.
+  better solutions are welcome.
+
+Obviously the r300 and via headers are no longer around ;-)
+
+Reason behind is that the drm headers can be used as a basic communications
+channel with the respective kernel modules. If more advanced functionality is
+required one can pull the specific libdrm_$driver which is free to pull
+additional files from the kernel.
+
+For example: nouveau has nouveau/nvif/*.h while vc4 has vc4/*.h
+
+If your driver is still in prototyping/staging state, consider moving the
+$driver_drm.h into $driver and _not_ installing it. An header providing opaque
+definitions and access [via $driver_drmif.h or similar] would be better fit.
+
+
+When and how to update these files
+----------------------------------
+Ideally on each libdrm release these will be kept in sync, with the latest
+released kernels. This way users won't need to provide local definitions.
+
+In order to update the files do the following:
+ - Switch to a Linux kernel tree/branch which is not rebased.
+For example: airlied/drm-next, drm-misc/XXX
+ - Install the headers via `make headers_install' to a separate location.
+ - Copy the drm header[s] + git add + git commit.
+ - Note: Your commit message must include:
+   a) Brief summary on the delta. If there's any change that looks like an
+API/ABI break one _must_ explicitly state why it's safe to do so.
+   b) "Generated using make headers_install."
+   c) "Generated from $tree/branch commit $sha"