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.