Page MenuHomeFreeBSD

Update linuxkpi with changes needed for drm-v5.1
ClosedPublic

Authored by ashafer_badland.io on Apr 29 2020, 1:25 AM.

Details

Summary

This changeset is part of a collaboration with greg_unrelenting.technology
to update kms-drm to 5.1.

Adds missing stub functions for a variety of subsystems: lockdep, scatterlist
pcie, nested mutexes, page iterators. Wherever possible we have tried to
call existing lkpi functions.

With this in base we can start looking at making an official drm-v5.1 branch.

Test Plan

This causes api breakage with sg_page_iter_dma_address. It looks like this
needs to accompany a bump in __FREEBSD_version. The older drm branches can
then be changed to fix build failures.

drm5.1 with these changes has been tested on haswell, modern intel, and amd.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This is my first phabricator review, please let me know if there is anything I missed.

This should NOT be committed without discussing the api breakage mentioned in the test plan

Changes look good. I'll have a closer look tonight.

With regards to the API breakage, there has to be an overlap period where both the old and the new API works in order to have time to update ports, and to have those updates propagate.

wulf added inline comments.
sys/compat/linuxkpi/common/include/linux/pci.h
960 ↗(On Diff #71134)

I just wonder. What is drm-kmod way to handle the fact that drm device is a grandchild of pci bus? Does it store a device_t of vgapci* as dev->dev.bsddev member?

sys/compat/linuxkpi/common/include/linux/pci.h
960 ↗(On Diff #71134)

I think so. I think drm_device holds a pointer to a linux pci_dev, which has a dev.bsddev member pointing to the bsd pci device.

sys/compat/linuxkpi/common/include/linux/pci.h
960 ↗(On Diff #71134)

We have this code which takes care of that:

	if (pdrv != NULL && pdrv->isdrm) {
		parent = device_get_parent(dev);
		dinfo = device_get_ivars(parent);
		device_set_ivars(dev, dinfo);
	} else {

Can you rebase this patch to focus only on the scatter-gather list changes. I've submitted the rest upstream now with some modifications.

sys/compat/linuxkpi/common/include/linux/scatterlist.h
96 ↗(On Diff #71134)

I think we need to do a simple cast here.

466 ↗(On Diff #71134)

Style:
return (true);

sys/compat/linuxkpi/common/include/linux/scatterlist.h
96 ↗(On Diff #71134)

I'm not sure I see what you mean. What part do you want to cast?

Are you talking about casting iter and passing it to for_each_sg_page to avoid code duplication?

sys/compat/linuxkpi/common/include/linux/scatterlist.h
96 ↗(On Diff #71134)

I mean, how can we make this work w/o breaking any APIs?

Addressed revision feedback and found a way to prevent api
breakage. sg_page_iter_dma_address was actually the section
that broke the code, so I used the idea of a cast there.

This built with drm 5.1, 5.0, and 4.16.

I'll have a closer look tomorrow.

sys/compat/linuxkpi/common/include/linux/scatterlist.h
69 ↗(On Diff #71242)

Could we simply do:

#define sg_dma_page_iter sg_page_iter
sys/compat/linuxkpi/common/include/linux/scatterlist.h
69 ↗(On Diff #71242)

We could, but it would require changing certain parts of drm to be FreeBSD specific, as they expect to be able to do sg_dma_page_iter->base.

https://github.com/amshafer/kms-drm/blob/drm-v5.1/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c#L320

At the moment there are only two such cases in 5.1, but my guess is the number will increase over time and not decrease. Although casting the function argument isn't ideal, the current patch will allow future drm code to be imported unchanged.

This revision was not accepted when it landed; it landed in state Needs Review.May 4 2020, 9:59 AM
This revision was automatically updated to reflect the committed changes.