Hi,
Sebastian Hesselbarth sebastian.hesselbarth@gmail.com writes:
- I think we could also drop the call to ->set_config since presumably an of-enabled driver grabbed any required info already from the dt.
[...]
I think this way we could still share encoder slaves across tons of platforms, only the init sequence (and specifically how they get at their
The "set_config" hook is just the way a DRM driver communicates those board-specific differences in the init sequence to the slave encoder driver. I don't think it would make sense to remove it unless we make sure it's being called elsewhere.
IMHO, the whole i2c encoder stuff is just wrong. Not any i2c slave driver is even really using its probe(). Everything is packed somewhere because it just worked.. this is at least a start.
Why is that? What do you mean by the probe hooks not being used? ch7006 and sil164 rely on it being called on initialization.
I suggest this to get merged to at least allow to have DT slaves, then start with improving tda998x as an example, then maybe rethink drm_slave_encoder completely, e.g.
- generalize the concept to SPI attached encoders, "internal" encoders..
drm_encoder_slave is bus-agnostic. drm_i2c_encoder_init() is just a helper function taking care of bus-specific details like the creation of the underlying I2C device object, which cannot be made bus-agnostic for obvious reasons. You're welcome to implement SPI and internal counterparts of drm_i2c_encoder_init().
- find a way to setup .encoder_type and .connector_type correctly
I guess encoder_type could be initialized correctly from the slave encoder_init() hook -- that hasn't been necessary until now because the DRM driver making use of the slave encoder has been expected to have some other means to find out encoder types [e.g. device-specific BIOS tables]. OTOH, I don't think that setting connector types is the slave encoder's business.
- have more common of_drm_blabla helpers
config data) would be different. That would also be extensible quite easily (*cough* intel platforms could setup encoder slaves from information out of the vbt *cough*).
[...]