Re: [Intel-gfx] [RFC 0/2] drm/connector: connector iterator with filtering

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 25 Nov 2022, Harry Wentland <harry.wentland@amd.com> 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@amd.com>
>
> Harry
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> Cc: Arun R Murthy <arun.r.murthy@intel.com>
>> Cc: Dave Airlie <airlied@gmail.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> 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