Hi Noralf,
> >> mode->flags) {
> >> dev_err(dev, "%pOF: panel-timing out of bounds\n",
> >> dev->of_node);
> >> return -EINVAL;
> >> }
> > With the display_timing => drm_display_mode I think the above is no
> > longer required.
> >
>
> I still need to verify the values to ensure that front_porch and
> sync_len are zero. Maybe I need a comment now to tell what I'm checking
> since I'm further away from the DT values:
>
> /*
> * Make sure width and height are set and that only back porch and
> * pixelclock are set in the other timing values. Also check that
> * width and height don't exceed the 16-bit value specified by MIPI DCS.
> */
Yes, that would be nice.
>
> >>
> >> /* 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)
> >> {
> > Not sure about the bus_flags argument here - seems misplaced.
> >
>
> I did the same as of_get_drm_display_mode(), don't panel drivers need
> the bus flags?
In my haste I missed the display_timing combines flags for the bus and
the mode - so yes it is needed.
>
> >> 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);
> >
> > This does a:
> > display_timing => video_mode => drm_display_display_mode
> >
> > We could do a:
> > display_timing => drm_display_mode.
> >
>
> I'll leave this to others to sort out. I want the function to look the
> same as of_get_drm_display_mode() and it uses videomode. If videomode
> goes away both can be fixed at the same time.
When I have dig myself out of the bridge hole I am in I may take a
look at this.
Sam
