Den 20.02.2022 11.04, skrev Sam Ravnborg:
> Hi Noralf,
> 
>>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct 
>>> drm_display_mode *mode)
>>> +{
>>> +   struct device *dev = dbidev->drm.dev;
>>> +   u32 width_mm = 0, height_mm = 0;
>>> +   struct display_timing timing;
>>> +   struct videomode vm;
>>> +   int ret;
>>> +
>>> +   ret = of_get_display_timing(dev->of_node, "panel-timing", &timing);
>>> +   if (ret) {
>>> +           dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", 
>>> dev->of_node, ret);
>>> +           return ret;
>>> +   }
>>> +
>>> +   videomode_from_timing(&timing, &vm);
>>> +
>>> +   if (!vm.hactive || vm.hfront_porch || vm.hsync_len ||
>>> +       (vm.hback_porch + vm.hactive) > 0xffff ||
>>> +       !vm.vactive || vm.vfront_porch || vm.vsync_len ||
>>> +       (vm.vback_porch + vm.vactive) > 0xffff ||
>>> +       vm.flags) {
>>> +           dev_err(dev, "%pOF: panel-timing out of bounds\n", 
>>> dev->of_node);
>>> +           return -EINVAL;
>>> +   }
>> We should have a helper that implements this. Maybe the display_timing
>> => display_mode helper could do it.
> 
> It would be nice with a drm_display_timing_to_mode() but that can come
> later - the comment above should not be understood that I consider it
> mandatory for this driver.
> 

I did consider adding an of_get_drm_panel_mode() fashioned after
of_get_drm_display_mode() but I didn't find any other driver that would
actually be able to use it and I would have to do some substraction to
get back the {h,v}front_porch values that I need and the optional pixel
clock calculation becomes more complex acting from a drm_display_mode so
I decided against it.

Looking at it now, what I could do is add a function like what
of_get_videomode() does for "display-timings":

/**
 * of_get_panel_videomode - get the panel-timing videomode from devicetree
 * @np: devicenode containing the panel-timing subnode
 * @vm: returns the videomode
 *
 * Returns:
 * Zero on success, negative error code on failure.
 **/
int of_get_panel_videomode(struct device_node *np, struct videomode *vm)
{
        struct display_timing timing;
        int ret;

        ret = of_get_display_timing(np, "panel-timing", &timing);
        if (ret)
                return ret;

        videomode_from_timing(&timing, vm);

        return 0;
}

This could also be used by panel-lvds and 2 fbdev drivers, the other
panel-timing users need/use the display_timing itself, some for bounds
checking.

Noralf.

Reply via email to