[PATCH 1/4] drm/panel: use fwnode based lookups for panel followers

[Jani Nikula]

On Mon, 09 Jun 2025, Doug Anderson <dianders at chromium.org> wrote:
> Hi,
>
> On Fri, Jun 6, 2025 at 2:06 AM Jani Nikula <jani.nikula at intel.com> wrote:
>>
>> Use firmware node based lookups for panel followers, to make the code
>> independent of OF and device tree, and make it work also for ACPI with
>> an appropriate _DSD.
>>
>> ASL example:
>>
>>         Package (0x02)
>>         {
>>                 "panel", \_SB.PCI0.GFX0.LCD0
>>         }
>>
>> Suggested-by: Heikki Krogerus <heikki.krogerus at linux.intel.com>
>> Cc: Neil Armstrong <neil.armstrong at linaro.org>
>> Cc: Jessica Zhang <jessica.zhang at oss.qualcomm.com>
>> Cc: Maxime Ripard <mripard at kernel.org>
>> Cc: Doug Anderson <dianders at chromium.org>
>> Cc: Lee Shawn C <shawn.c.lee at intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  drivers/gpu/drm/drm_panel.c | 39 +++++++++++++++++++++++++++++--------
>>  1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index fee65dc65979..3eb0a615f7a9 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -473,17 +473,40 @@ int of_drm_get_panel_orientation(const struct device_node *np,
>>  EXPORT_SYMBOL(of_drm_get_panel_orientation);
>>  #endif
>>
>> -static struct drm_panel *of_find_panel(struct device *follower_dev)
>> +/* Find panel by fwnode */
>> +static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
>
> nit: It might be worth adding a comment that says it should be
> identical to of_drm_find_panel() since that has a much richer
> kerneldoc that talks about the corner cases.

Agreed.

I'm actually wondering if it would be possible to implement
of_drm_find_panel() like this (as a follow-up change):

struct drm_panel *of_drm_find_panel(const struct device_node *np)
{
	const struct fwnode_handle *fwnode = of_fwnode_handle(np);

	return find_panel_by_fwnode(fwnode);
}

But I'd rather leave that out for now.

>
>>  {
>> -       struct device_node *panel_np;
>>         struct drm_panel *panel;
>>
>> -       panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
>> -       if (!panel_np)
>> +       if (!fwnode_device_is_available(fwnode))
>>                 return ERR_PTR(-ENODEV);
>>
>> -       panel = of_drm_find_panel(panel_np);
>> -       of_node_put(panel_np);
>> +       mutex_lock(&panel_lock);
>> +
>> +       list_for_each_entry(panel, &panel_list, list) {
>> +               if (dev_fwnode(panel->dev) == fwnode) {
>> +                       mutex_unlock(&panel_lock);
>> +                       return panel;
>> +               }
>> +       }
>> +
>> +       mutex_unlock(&panel_lock);
>> +
>> +       return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +/* Find panel by follower device */
>> +static struct drm_panel *find_panel_by_dev(struct device *follower_dev)
>> +{
>> +       struct fwnode_handle *fwnode;
>> +       struct drm_panel *panel;
>> +
>> +       fwnode = fwnode_find_reference(dev_fwnode(follower_dev), "panel", 0);
>> +       if (IS_ERR_OR_NULL(fwnode))
>
> nit: why IS_ERR_OR_NULL() instead of IS_ERR()? The kerneldoc for
> fwnode_find_reference() doesn't mention anything about it returning a
> NULL value in any cases...

Will fix.

> Other than the nits, this looks reasonable to me.
>
> Reviewed-by: Douglas Anderson <dianders at chromium.org>
>
> I no longer have any easy access to hardware where panel-follower is
> truly necessary, but I can at least see the panel-follower calls
> getting made on sc7180-trogdor-lazor, so the of->fwnode conversion
> stuff must be working.
>
> Tested-by: Douglas Anderson <dianders at chromium.org>

Thanks for the review and testing, much appreciated!

BR,
Jani.


-- 
Jani Nikula, Intel