On Tue, 5 Jan 2016 20:38:17 +0000 Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote: > Some comments from an ARM architecture point of view. I haven't > reviewed it from a DRM point of view yet. Anyway, thanks for your explanations and remarks. > > +static void de2_crtc_enable(struct drm_crtc *crtc) > > +{ ... > > + clk_set_rate(lcd->clk, mode->clock * 1000); > > What if the clock can't support the rate? The function enabling the CRTC has no return code, so the screen would be blurred. But this would not occur: the video PLL is in the range 30..600MHz. > > +static int de2_hdmi_connector_mode_valid(struct drm_connector *connector, > > + struct drm_display_mode *mode) > > +{ > > + if (!drm_match_cea_mode(mode)) > > + return MODE_NOMODE; > > Maybe detect modes with a zero clock here instead? We have no documentation about the HDMI hardware and only the CEA modes are handled in Allwinner's driver. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel