Hi Abhinav, > Based on the discussion in this thread [1] , it seems like having drm_encoder > as a pointer seems to have merits for both of us and also in agreement with > the folks on this thread and has a better chance of an ack. > > However drm_connector is not. > > I had a brief look at your implementation. Any reason you need to go > through intel_connector here and not drm_writeback_connector directly? > > The reason I ask is that I see you are not using prepare_writeback_job hook. > If you use that, you can use the drm_writeback_connector passed on from > there instead of looping through your list like you are doing in > intel_find_writeback_connector. > > Also, none of the other entries of struct intel_connector are being used for > the writeback implementation. So does the drm_writeback_connector in > your implementation need to be an intel_connector when its anyway not > using other fields? Why cant it be just stored as a drm_writeback_connector > itself in your struct intel_wd. The reason we can't do that is i915 driver always assumes that all connectors present in device list is an intel connector and since drm_writeback_connector even though a faux connector in it's initialization calls drm_connector_init() and gets added to the drm device list and hence the i915 driver also expects a corresponding intel connector to go with it. In case I do try to make writeback connector standalone a lot of checks, a lot will have to be added all around the driver as there could be a chance that one of the drm_connector in the drm device list may not be an intel_connector. Yes right now not all fields of intel_connector are being used but we will be working on filling them as we add more functionality to writeback for example introducing content protection. So even if I do float the patch series with just drm_encoder as pointer it won't help us with our implementation if drm_connector isn't a pointer too. Regards, Suraj Kandpal
