On Fri, 25 Nov 2022, Harry Wentland <harry.wentland@xxxxxxx> wrote: > On 10/5/22 06:51, Jani Nikula wrote: >> Currently i915 assumes all drm_connectors it encounters are embedded in >> intel_connectors that i915 allocated. The drm_writeback_connector forces >> a design where this is not the case; we can't provide our own connector, >> and writeback embeds the drm_connector it initializes itself. >> >> To use drm writeback, none of the i915 connector iteration could assume >> the drm connector is embedded in intel_connector. Checking this is >> tedious, and would require an intermediate step with >> drm_connector. Here's an idea I came up with; filtering at the drm >> connector iterator level with a caller supplied function. Not too much >> code, and could be used for other things as well. >> > > We've been trying to hook up drm_writeback_connector in amdgpu and > this would be really helpful. I've had to do liberal sprinkling > of "!= DRM_MODE_CONNECTOR_WRITEBACK" all over the place. > >> Mind you, we'd still much rather modify drm writeback to allow passing >> the connector i915 allocated, instead of the current midlayer design >> that forces drivers to a certain model. Working around this is a bunch >> of error prone and tedious code that we really could do without. >> > > I think this would be even better but also be much more work and impact > every driver that implements writeback. FWIW, there was no way for me > to add writeback connector handling without KASAN. Interpreting the > connector wrong in one place leads to memory corruption and > undefined behavior and is almost impossible to spot without KASAN. Laurent, I once again plead - could we please reconsider this and give drivers the option to allocate and provide the drm_connector to writeback themselves? All things considered I think that would be the safer option. BR, Jani. > > This series is > Acked-by: Harry Wentland <harry.wentland@xxxxxxx> > > Harry > >> >> BR, >> Jani. >> >> >> Cc: Arun R Murthy <arun.r.murthy@xxxxxxxxx> >> Cc: Dave Airlie <airlied@xxxxxxxxx> >> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >> Cc: Suraj Kandpal <suraj.kandpal@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> >> Jani Nikula (2): >> drm/connector: add connector list iteration with filtering >> drm/i915: iterate intel_connectors only >> >> drivers/gpu/drm/drm_connector.c | 57 +++++++++++++++---- >> drivers/gpu/drm/i915/display/intel_display.c | 3 +- >> .../drm/i915/display/intel_display_types.h | 7 +++ >> drivers/gpu/drm/i915/display/intel_dp.c | 6 +- >> drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- >> drivers/gpu/drm/i915/display/intel_hdcp.c | 3 +- >> drivers/gpu/drm/i915/display/intel_hotplug.c | 12 ++-- >> .../drm/i915/display/intel_modeset_setup.c | 6 +- >> drivers/gpu/drm/i915/display/intel_opregion.c | 9 ++- >> include/drm/drm_connector.h | 9 +++ >> 10 files changed, 89 insertions(+), 26 deletions(-) >> > -- Jani Nikula, Intel Open Source Graphics Center