Re: userptr support in drm drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



At least for Radeon and Amdgpu the current situation is actually what we want.

However I still find it confusing
that full userptr support is under #if defined(CONFIG_MMU_NOTIFIER),
and not under #if defined(CONFIG_RADEON_USERPTR), resp. #if
defined(CONFIG_AMDGPU_USERPTR). It means that full userptr support may
be included even if these options are disabled.
Which is perfectly fine. Userptr support should be enabled when MMU_NOTIFIER is available.

How this becomes available is a different story. You can explicitly enable it which then pulls in the MMU_NOTIFIER dependency or you just enable it when you have the notfier anyway because of some other dependency.

That we have two options doing the same is just a matter of branching of amdgpu from radeon and not cleaning up the configuration options. So feel free to cleaning this up and write a patch which makes pulling in MMU_NOTIFIER as a general DRM option.

Regards,
Christian.

Am 16.02.2016 um 20:58 schrieb Jean Delvare:
Hi all,

While checking the openSUSE kernel configuration, I noticed a couple
oddities regarding userptr support in the i915, radeon and amdgpu drm
drivers. I'll like to discuss the current situation and come up with an
agreement on how this could be cleaned up.

Firstly, i915. This driver has userptr code under #if
defined(CONFIG_MMU_NOTIFIER), however it neither selects this option nor
depends on it. Given that CONFIG_MMU_NOTIFIER is not a user-visible
option (it can only be selected by other kernel configuration options),
it means that you get full userptr support or not depending on other
unrelated kernel options. This isn't good.

Secondly, radeon and amdgpu. They are slightly better in that they have
Kconfig options selecting CONFIG_MMU_NOTIFIER (DRM_RADEON_USERPTR and
DRM_AMDGPU_USERPTR respectively.) However I still find it confusing
that full userptr support is under #if defined(CONFIG_MMU_NOTIFIER),
and not under #if defined(CONFIG_RADEON_USERPTR), resp. #if
defined(CONFIG_AMDGPU_USERPTR). It means that full userptr support may
be included even if these options are disabled.

I am not too familiar with MMU and this userptr stuff, but from where I
stand, only two approaches make sense:

* Either there is a good reason why people may want to disable full
   userptr support. In this case options CONFIG_RADEON_USERPTR and
   CONFIG_AMDGPU_USERPTR should really enable the code in question, it
   should not be built without these options. And a similar option
   should be introduced for the i915 driver to the same effect. Or
   alternatively a single option for the whole DRM subsystem may make
   more sense, as I doubt anyone would want to enable support in one
   driver and disable it in another.

* Or there is no specific reason why people would want to disable full
   userptr support, in which case options CONFIG_RADEON_USERPTR and
   CONFIG_AMDGPU_USERPTR should be removed and all 3 drivers should
   unconditionally select CONFIG_MMU_NOTIFIER.

If the sole purpose of these these settings is for development or
debugging purposes, I'd go for option 1 with a run-time option to
disable full userptr (drm.userptr=ro or some such.)

As a general rule, fewer configuration options is better.

Once a decision is made, I volunteer to write the patches.

Thanks,

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux