Page MenuHomeFreeBSD

linuxkpi: Add i2c support
AcceptedPublic

Authored by manu on Thu, Nov 18, 8:21 AM.

Details

Reviewers
hselasky
Group Reviewers
linuxkpi
Summary

Add i2c support to linuxkpi. This is needed by drm-kmod.
For every i2c_adapter added by i2c_add_adapter we add a child to the
device named "lkpi_iic". This child handle the conversion between
Linux i2c_msgs to FreeBSD iic_msgs.
For every i2c_adapter added by i2c_bit_add_bus we add a child to the
device named "lkpi_iicbb". This child handle the conversion between
Linux i2c_msgs to FreeBSD iic_msgs.
With the help of iic(4), this expose the i2c controller to userspace
allowing a user to query DDC information from a monitor.
e.g.: i2c -f /dev/iic0 -a 0x28 -c 128 -d r
will query the standard EDID from the monitor if plugged.

The bitbang part (lkpi_iicbb) isn't tested at all for now as I don't have
compatible hardware (all my hardware have native i2c controller).

Tested on: Intel Skylake, AMD Picasso, AMD Polaris (arm64).
Sponsored by: Beckhoff Automation GmbH & Co. KG

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 42864
Build 39752: arc lint + arc unit

Event Timeline

manu requested review of this revision.Thu, Nov 18, 8:21 AM

To use with https://github.com/freebsd/drm-kmod/commit/0d7cacc62d4277486bfff79e3bdac66045de2667
I plan to send a mail for a CFT as some part of this code isn't tested (lkpi_iicbb part).

hselasky added a subscriber: hselasky.

Looks good.

sys/compat/linuxkpi/common/src/linux_i2c.c
151

For small number of nmsgs you may want to allocate on the stack? Depends how frequently the function is used.

This revision is now accepted and ready to land.Thu, Nov 18, 9:06 AM

You're missing a

MODULE_DEPEND(linuxkpi, iicbus, IICBUS_MINVER, IICBUS_PREFVER, IICBUS_MAXVER);

line — with the patch as-is I get

[257] link_elf_obj: symbol iicbus_driver undefined
[257] linker_load_file: /boot/kernel/linuxkpi.ko - unsupported file type

and so on (on my config where iicbus is not built-in).


On my Vega 56, the bus is not working — nothing probes, edid command results in i2c: ioctl(I2CRDWR) failed: Input/output error. compat.linuxkpi.amdgpu_hw_i2c was 0 by default but booting with it set to 1 didn't change anything.

You're missing a

MODULE_DEPEND(linuxkpi, iicbus, IICBUS_MINVER, IICBUS_PREFVER, IICBUS_MAXVER);

line — with the patch as-is I get

[257] link_elf_obj: symbol iicbus_driver undefined
[257] linker_load_file: /boot/kernel/linuxkpi.ko - unsupported file type

and so on (on my config where iicbus is not built-in).

Oops, thanks I'll update the patch.


On my Vega 56, the bus is not working — nothing probes, edid command results in i2c: ioctl(I2CRDWR) failed: Input/output error. compat.linuxkpi.amdgpu_hw_i2c was 0 by default but booting with it set to 1 didn't change anything.

But amdgpu found the screen or nothing works ?
May I ask for the pci id ?

wulf added inline comments.
sys/compat/linuxkpi/common/src/linux_i2c.c
151

For small number of nmsgs you may want to allocate on the stack? Depends how frequently the function is used.

FreeBSD struct iic_msg definition

/* Designed to be compatible with linux's struct i2c_msg */
struct iic_msg
{
        uint16_t        slave;
        uint16_t        flags;
#define IIC_M_WR        0       /* Fake flag for write */
#define IIC_M_RD        0x0001  /* read vs write */
#define IIC_M_NOSTOP    0x0002  /* do not send a I2C stop after message */
#define IIC_M_NOSTART   0x0004  /* do not send a I2C start before message */
        uint16_t        len;    /* msg length */
        uint8_t *       buf;
};

looks very close to proposed LKPI struct i2c_msg definition

#define I2C_M_RD        0x0001
#define I2C_M_NOSTART   0x0002
#define I2C_M_STOP      0x0004
struct i2c_msg {
        uint16_t addr;
        uint16_t flags;
        uint16_t len;
        uint8_t *buf;
};

I think that malloc can be converted to cast here

In D33053#746767, @manu wrote:

But amdgpu found the screen or nothing works ?
May I ask for the pci id ?

The screen (over DP 1.2) works great as usual, nothing broke. ID is 1002:687F.

You're missing a

MODULE_DEPEND(linuxkpi, iicbus, IICBUS_MINVER, IICBUS_PREFVER, IICBUS_MAXVER);

line — with the patch as-is I get

[257] link_elf_obj: symbol iicbus_driver undefined
[257] linker_load_file: /boot/kernel/linuxkpi.ko - unsupported file type

and so on (on my config where iicbus is not built-in).


On my Vega 56, the bus is not working — nothing probes, edid command results in i2c: ioctl(I2CRDWR) failed: Input/output error.

And you have that for all iic node in /dev ? It's normal to have this for uninitialized hardware (like some unconnected edp or something like that).

compat.linuxkpi.amdgpu_hw_i2c was 0 by default but booting with it set to 1 didn't change anything.

amdgpu_hw_i2c is not taken into account if the chip has DC support (so anything > Sea Island).
I hadn't realized that before so I might have a way to test lkpi_iicbb with my hardward.

sys/compat/linuxkpi/common/src/linux_i2c.c
151

Mhm true, it seems that I just need to rewrite the flags part.

In D33053#747336, @manu wrote:

amdgpu_hw_i2c is not taken into account if the chip has DC support (so anything > Sea Island).
I hadn't realized that before so I might have a way to test lkpi_iicbb with my hardward.

yeah just found that out via some printf debugging :) So the interesting thing is, all the linuxkpi devices exposed to userspace via /dev/iicN hit amdgpu_dm_i2c_xfer, but the driver's internal handling of monitor hotplug actually uses drm_dp_i2c_xfer instead (no messages logged from DC!).
amdgpu_dm_i2c_xfer goes down the hardware path, and ends up with I2C_CHANNEL_OPERATION_ENGINE_BUSY on the second operation.