| Message ID | 8d92019b6ae730d6dc019e03fbc103645a2cf203.1664966047.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/connector: connector iterator with filtering | expand |
Hi Jani, Thank you for the patch. 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@intel.com> > Cc: Suraj Kandpal <suraj.kandpal@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > 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); > + > /** > * 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); I would have made this a static inline function in include/drm/drm_connector.h. Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > @@ -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);
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@intel.com> > Cc: Suraj Kandpal <suraj.kandpal@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > 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. -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 >
On Thu, 15 Dec 2022, Daniel Vetter <daniel@ffwll.ch> 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@intel.com> >> Cc: Suraj Kandpal <suraj.kandpal@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> 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 >>
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); + /** * 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);
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@intel.com> Cc: Suraj Kandpal <suraj.kandpal@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_connector.c | 57 ++++++++++++++++++++++++++------- include/drm/drm_connector.h | 9 ++++++ 2 files changed, 54 insertions(+), 12 deletions(-)