On Thu, 15 Dec 2022, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Wed, Oct 05, 2022 at 01:51:43PM +0300, Jani Nikula wrote:
>> Add new function drm_connector_list_iter_filter_begin() to initialize
>> connector list iterator with a filter function. Subsequent iteration on
>> the list will only return connectors on which the filter function
>> returns true.
>>
>> Cc: Arun R Murthy <arun.r.murthy@xxxxxxxxx>
>> Cc: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_connector.c | 57 ++++++++++++++++++++++++++-------
>> include/drm/drm_connector.h | 9 ++++++
>> 2 files changed, 54 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index e3142c8142b3..d54b4b54cecb 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -762,6 +762,29 @@ static struct lockdep_map connector_list_iter_dep_map = {
>> };
>> #endif
>>
>> +/**
>> + * drm_connector_list_iter_filter_begin - initialize a connector_list iterator with filter
>> + * @dev: DRM device
>> + * @iter: connector_list iterator
>> + * @filter: connector filter function
>> + * @filter_context: context to be passed to the filter function
>> + *
>> + * Same as drm_connector_list_iter_begin(), but sets up the iterator to only
>> + * return connectors where filter(connector) returns true.
>> + */
>> +void drm_connector_list_iter_filter_begin(struct drm_device *dev,
>> + struct drm_connector_list_iter *iter,
>> + drm_connector_list_iter_filter_fn filter,
>> + void *filter_context)
>> +{
>> + iter->dev = dev;
>> + iter->conn = NULL;
>> + iter->filter = filter;
>> + iter->filter_context = filter_context;
>> + lock_acquire_shared_recursive(&connector_list_iter_dep_map, 0, 1, NULL, _RET_IP_);
>> +}
>> +EXPORT_SYMBOL(drm_connector_list_iter_filter_begin);
>
> So maybe I'm missing the obvious, but can't we just put a for_each_fi
> right after the for_each_connector_iter?
>
> And then maybe provide a default filter to kick out connectors and maybe a
> macro that does the filtered iteration? The iter_begin/next/end is already
> fairly tricky pattern compared to plain for_each macro, I think we should
> try hard to not complicate it further with lots of variants if that's not
> absolutely necessary.
Sorry, dropped the ball here, and lost sight of it.
You mean not have any of this in drm core, and just add the
for_each_if() in local wrappers everywhere?
BR,
Jani.
> -Daniel
>
>
>> +
>> /**
>> * drm_connector_list_iter_begin - initialize a connector_list iterator
>> * @dev: DRM device
>> @@ -775,9 +798,7 @@ static struct lockdep_map connector_list_iter_dep_map = {
>> void drm_connector_list_iter_begin(struct drm_device *dev,
>> struct drm_connector_list_iter *iter)
>> {
>> - iter->dev = dev;
>> - iter->conn = NULL;
>> - lock_acquire_shared_recursive(&connector_list_iter_dep_map, 0, 1, NULL, _RET_IP_);
>> + drm_connector_list_iter_filter_begin(dev, iter, NULL, NULL);
>> }
>> EXPORT_SYMBOL(drm_connector_list_iter_begin);
>>
>> @@ -800,15 +821,8 @@ __drm_connector_put_safe(struct drm_connector *conn)
>> schedule_work(&config->connector_free_work);
>> }
>>
>> -/**
>> - * drm_connector_list_iter_next - return next connector
>> - * @iter: connector_list iterator
>> - *
>> - * Returns: the next connector for @iter, or NULL when the list walk has
>> - * completed.
>> - */
>> -struct drm_connector *
>> -drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
>> +static struct drm_connector *
>> +__drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
>> {
>> struct drm_connector *old_conn = iter->conn;
>> struct drm_mode_config *config = &iter->dev->mode_config;
>> @@ -836,6 +850,25 @@ drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
>>
>> return iter->conn;
>> }
>> +
>> +/**
>> + * drm_connector_list_iter_next - return next connector
>> + * @iter: connector_list iterator
>> + *
>> + * Returns: the next connector for @iter, or NULL when the list walk has
>> + * completed.
>> + */
>> +struct drm_connector *
>> +drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
>> +{
>> + struct drm_connector *connector;
>> +
>> + while ((connector = __drm_connector_list_iter_next(iter)) &&
>> + iter->filter && !iter->filter(connector, iter->filter_context))
>> + ;
>> +
>> + return connector;
>> +}
>> EXPORT_SYMBOL(drm_connector_list_iter_next);
>>
>> /**
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 56aee949c6fa..497b98197d3a 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -1868,6 +1868,9 @@ struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
>> void drm_mode_put_tile_group(struct drm_device *dev,
>> struct drm_tile_group *tg);
>>
>> +typedef bool (*drm_connector_list_iter_filter_fn)(const struct drm_connector *connector,
>> + void *filter_context);
>> +
>> /**
>> * struct drm_connector_list_iter - connector_list iterator
>> *
>> @@ -1886,10 +1889,16 @@ struct drm_connector_list_iter {
>> /* private: */
>> struct drm_device *dev;
>> struct drm_connector *conn;
>> + drm_connector_list_iter_filter_fn filter;
>> + void *filter_context;
>> };
>>
>> void drm_connector_list_iter_begin(struct drm_device *dev,
>> struct drm_connector_list_iter *iter);
>> +void drm_connector_list_iter_filter_begin(struct drm_device *dev,
>> + struct drm_connector_list_iter *iter,
>> + drm_connector_list_iter_filter_fn filter,
>> + void *filter_context);
>> struct drm_connector *
>> drm_connector_list_iter_next(struct drm_connector_list_iter *iter);
>> void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
>> --
>> 2.34.1
>>
--
Jani Nikula, Intel Open Source Graphics Center
