> 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.
Scratch that, since videomode is to be avoided I tried adding a
drm_display_mode function and it didn't complicate matter as I though it
would so I'll do that instead:
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;
u16 hback_porch, vback_porch;
struct videomode vm;
int ret;
ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL);
if (ret) {
dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n",
dev->of_node, ret);
return ret;
}
mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
hback_porch = mode->htotal - mode->hsync_end;
vback_porch = mode->vtotal - mode->vsync_end;
if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay)
> 0xffff ||
mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay)
> 0xffff ||
mode->flags) {
dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node);
return -EINVAL;
}
/* The driver doesn't use the pixel clock but it is mandatory so fake
one if not set */
if (!mode->pixelclock)
mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000;
dbidev->top_offset = vback_porch;
dbidev->left_offset = hback_porch;
return 0;
}
int of_get_drm_panel_display_mode(struct device_node *np,
struct drm_display_mode *dmode, u32 *bus_flags)
{
u32 width_mm = 0, height_mm = 0;
struct display_timing timing;
struct videomode vm;
int ret;
ret = of_get_display_timing(np, "panel-timing", &timing);
if (ret)
return ret;
videomode_from_timing(&timing, vm);
memset(dmode, 0, sizeof(*dmode));
drm_display_mode_from_videomode(&vm, dmode);
if (bus_flags)
drm_bus_flags_from_videomode(&vm, bus_flags);
ret = of_property_read_u32(np, "width-mm", &width_mm);
if (ret && ret != -EINVAL)
return ret;
ret = of_property_read_u32(np, "height-mm", &height_mm);
if (ret && ret != -EINVAL)
return ret;
mode->width_mm = width_mm;
mode->height_mm = height_mm;
drm_mode_debug_printmodeline(dmode);
return 0;
}
EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
Noralf.
