Hi, Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx> 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*). >> >[...]
Attachment:
pgpC478zu8VkX.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel