diff mbox

[2/2] drm/sun4i: add lvds mode_valid function

Message ID 1519840432-61489-2-git-send-email-giulio.benetti@micronovasrl.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Giulio Benetti Feb. 28, 2018, 5:53 p.m. UTC
mode_valid function is missing for lvds.

Add it based on rgb model, also setting up dclk_min_div and dclk_max_div

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_lvds.c | 55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Maxime Ripard March 1, 2018, 9:57 a.m. UTC | #1
On Wed, Feb 28, 2018 at 06:53:52PM +0100, Giulio Benetti wrote:
>  static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
>  	.get_modes	= sun4i_lvds_get_modes,
> +	.mode_valid	= sun4i_lvds_mode_valid,
>  };

This should be on the encoder, not the connector.

Maxime
Giulio Benetti March 2, 2018, 11:42 a.m. UTC | #2
Hi,

Il 01/03/2018 10:57, Maxime Ripard ha scritto:
> On Wed, Feb 28, 2018 at 06:53:52PM +0100, Giulio Benetti wrote:
>>   static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
>>   	.get_modes	= sun4i_lvds_get_modes,
>> +	.mode_valid	= sun4i_lvds_mode_valid,
>>   };
> 
> This should be on the encoder, not the connector.

I've seen it is bound to connector in rgb and to encoder in hdmi.
Is it correct rgb mode_valid under connector funcs?
Otherwise I send a patch also for that one.

> 
> Maxime
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Maxime Ripard March 2, 2018, 2:37 p.m. UTC | #3
On Fri, Mar 02, 2018 at 12:42:14PM +0100, Giulio Benetti wrote:
> Hi,
> 
> Il 01/03/2018 10:57, Maxime Ripard ha scritto:
> > On Wed, Feb 28, 2018 at 06:53:52PM +0100, Giulio Benetti wrote:
> > >   static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
> > >   	.get_modes	= sun4i_lvds_get_modes,
> > > +	.mode_valid	= sun4i_lvds_mode_valid,
> > >   };
> > 
> > This should be on the encoder, not the connector.
> 
> I've seen it is bound to connector in rgb and to encoder in hdmi.
> Is it correct rgb mode_valid under connector funcs?
> Otherwise I send a patch also for that one.

This would need to be fixed as well. Bridges attach to encoder, not
connectors, so if you ever have a bridge connected to the RGB output
(like on the A13-Olinuxino), mode_valid isn't called at the moment.

Maxime
Giulio Benetti March 2, 2018, 4:50 p.m. UTC | #4
Hi,

Il 02/03/2018 15:37, Maxime Ripard ha scritto:
> On Fri, Mar 02, 2018 at 12:42:14PM +0100, Giulio Benetti wrote:
>> Hi,
>>
>> Il 01/03/2018 10:57, Maxime Ripard ha scritto:
>>> On Wed, Feb 28, 2018 at 06:53:52PM +0100, Giulio Benetti wrote:
>>>>    static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
>>>>    	.get_modes	= sun4i_lvds_get_modes,
>>>> +	.mode_valid	= sun4i_lvds_mode_valid,
>>>>    };
>>>
>>> This should be on the encoder, not the connector.
>>
>> I've seen it is bound to connector in rgb and to encoder in hdmi.
>> Is it correct rgb mode_valid under connector funcs?
>> Otherwise I send a patch also for that one.
> 
> This would need to be fixed as well. Bridges attach to encoder, not
> connectors, so if you ever have a bridge connected to the RGB output
> (like on the A13-Olinuxino), mode_valid isn't called at the moment.

Ok, I will do the same for rgb and submit a patchset,
need some time to test both lvds and rgb.
diff mbox

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
index be3f14d..75223ee 100644
--- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
+++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
@@ -46,8 +46,63 @@  static int sun4i_lvds_get_modes(struct drm_connector *connector)
 	return drm_panel_get_modes(tcon->panel);
 }
 
+static int sun4i_lvds_mode_valid(struct drm_connector *connector,
+				struct drm_display_mode *mode)
+{
+	struct sun4i_lvds *lvds = drm_connector_to_sun4i_lvds(connector);
+	struct sun4i_tcon *tcon = lvds->tcon;
+	u32 hsync = mode->hsync_end - mode->hsync_start;
+	u32 vsync = mode->vsync_end - mode->vsync_start;
+	unsigned long rate = mode->clock * 1000;
+	long rounded_rate;
+
+	DRM_DEBUG_DRIVER("Validating modes...\n");
+
+	if (hsync < 1)
+		return MODE_HSYNC_NARROW;
+
+	if (hsync > 0x3ff)
+		return MODE_HSYNC_WIDE;
+
+	if ((mode->hdisplay < 1) || (mode->htotal < 1))
+		return MODE_H_ILLEGAL;
+
+	if ((mode->hdisplay > 0x7ff) || (mode->htotal > 0xfff))
+		return MODE_BAD_HVALUE;
+
+	DRM_DEBUG_DRIVER("Horizontal parameters OK\n");
+
+	if (vsync < 1)
+		return MODE_VSYNC_NARROW;
+
+	if (vsync > 0x3ff)
+		return MODE_VSYNC_WIDE;
+
+	if ((mode->vdisplay < 1) || (mode->vtotal < 1))
+		return MODE_V_ILLEGAL;
+
+	if ((mode->vdisplay > 0x7ff) || (mode->vtotal > 0xfff))
+		return MODE_BAD_VVALUE;
+
+	DRM_DEBUG_DRIVER("Vertical parameters OK\n");
+
+	tcon->dclk_min_div = 7;
+	tcon->dclk_max_div = 18;
+	rounded_rate = clk_round_rate(tcon->dclk, rate);
+	if (rounded_rate < rate)
+		return MODE_CLOCK_LOW;
+
+	if (rounded_rate > rate)
+		return MODE_CLOCK_HIGH;
+
+	DRM_DEBUG_DRIVER("Clock rate OK\n");
+
+	return MODE_OK;
+}
+
 static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
 	.get_modes	= sun4i_lvds_get_modes,
+	.mode_valid	= sun4i_lvds_mode_valid,
 };
 
 static void