[PATCH RFC 2/2] drm: sunxi: Add a basic DRM driver for Allwinner DE2

[Jean-Francois Moine]

On Tue, 5 Jan 2016 20:38:17 +0000
Russell King - ARM Linux <linux at arm.linux.org.uk> 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/