agp: Gracefully handle make_dev() errors if /dev/agpgart already exists

[markj]

Summary

We have some long-standing bug reports about this. Currently the driver
will panic if multiple agp devices are present. The prevailaing
workaround is simply to disable all but one using device hints, so let's
modify the driver to do that automatically rather than panicking.

This does not go as far as the patch in PR 187015, which assigns unit
numbers to all but the first device. I am not sure if that approach is
really useful and don't have any hardware to try it, but if so I can
modify the patch.

PR: 187015

[markj]

Also need to set MAKEDEV_CHECKNAME

[kib]

Is there any use of agp attach without /dev/agpgart? I think it is only used by very old chipsets, newer Intels with pseudo agp interfaces use its own apperture page management.

IMO if we pretend that the driver is useful, it should create units as mentioned in summary. Or disable attach of any but first unit. But I do not object against the patch, I believe that it is effectively does not affect usability, and indeed prevents panics on old machines.

[ota_j.email.ne.jp]

I will test and report back.

[ota_j.email.ne.jp]

I patched to 13.0-RELEASE.
I rebuilt and installed the new kernel to the specific machine.
I removed hint.agp.1.disabled="1" from /boot/loader.conf.
I verified the new kernel with the patch booted okay after removing the config.

[markj]

In D33068#747080, @kib wrote:

IMO if we pretend that the driver is useful, it should create units as mentioned in summary. Or disable attach of any but first unit.

What is the canonical way to disable attach of all but the first unit? Is there some newbus mechanism for this?

But I do not object against the patch, I believe that it is effectively does not affect usability, and indeed prevents panics on old machines.

This is exactly the intent. If there is some desire to support multiple agpgart devices then the driver can create /dev/agpgart<unit> and make /dev/agpgart an alias of /dev/agpgart0.

[imp]

In D33068#747699, @markj wrote:

In D33068#747080, @kib wrote:

IMO if we pretend that the driver is useful, it should create units as mentioned in summary. Or disable attach of any but first unit.

What is the canonical way to disable attach of all but the first unit? Is there some newbus mechanism for this?

I think 'if (device_get_unit(dev) != 0) return ENXIO;' in the attach routine. That produces an ugly error though,
so often times I've seen in private code 'return 0' and have no resources created for the device, perhaps apart
from activating the resources automatically assigned to the device.

But I do not object against the patch, I believe that it is effectively does not affect usability, and indeed prevents panics on old machines.

This is exactly the intent. If there is some desire to support multiple agpgart devices then the driver can create /dev/agpgart<unit> and make /dev/agpgart an alias of /dev/agpgart0.

Is there anything that uses that?

[kib]

There is a reason why it is probably much better to expose all agpgart nodes, instead of some random node. It is because actually these AGP devices are not interchangeable.

As I understand, two agp devices exist for Intel chipsets that both have integrated graphics, and offer AGP port for external GPU card. There, integrated graphics exports its GTT as AGP device, but it is not. So depending on GPU you use, you need either pseudo-AGP or real AGP. Of course this is all moot because it is only relevant for ancient hardware.

Might be agpgart(4) should be removed.

[markj]

Go a bit further and create /dev/agpgart%d for all devices, and create the alias
for the device with unit number 0.

[markj]

In D33068#747901, @kib wrote:

There is a reason why it is probably much better to expose all agpgart nodes, instead of some random node. It is because actually these AGP devices are not interchangeable.

As I understand, two agp devices exist for Intel chipsets that both have integrated graphics, and offer AGP port for external GPU card. There, integrated graphics exports its GTT as AGP device, but it is not. So depending on GPU you use, you need either pseudo-AGP or real AGP. Of course this is all moot because it is only relevant for ancient hardware.

Ok, I just went ahead and started creating a device node for each attach.

Might be agpgart(4) should be removed.

Perhaps, I'm not sure exactly what the criteria are.

[kib]

sys/dev/agp/agp.c

272

error == 0 && unit == 0 ?

[markj]

Attempt to create the alias iff the device unit number is 0.

[ota_j.email.ne.jp]

I tested the new patch with 13.0-RELEASE and confirmed crash isn't happening.