From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 31 Dec 2014 12:59:07 +0100
Subject: [U-Boot] [PATCH 09/17] sunxi: video: Add lcd output support
In-Reply-To: <1419860621.13595.81.camel@hellion.org.uk>
References: <1419447989-21959-1-git-send-email-hdegoede@redhat.com>	
	<1419447989-21959-10-git-send-email-hdegoede@redhat.com>
	<1419860621.13595.81.camel@hellion.org.uk>
Message-ID: <54A3E50B.4080507@redhat.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi,

On 29-12-14 14:43, Ian Campbell wrote:
> On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
>
>> +static void sunxi_lcdc_panel_enable(void)
>> +{
>> +	int pin;
>> +
>> +	/*
>> +	 * Start with backlight disabled to avoid the screen flashing to
>> +	 * white while the lcd inits.
>> +	 */
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_EN);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_backlight_enable");
>
> You request this and the backlight pwm again in backlight_enable. Are
> such multiple requests OK?

No, not when using the device_model, I've removed the 2nd gpio_request
call done for both from backlight_enable. I'll send a v2 with this fixed.

>> +		gpio_direction_output(pin, 0);
>> +	}
>> +
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_BL_PWM);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_backlight_pwm");
>> +		/* backlight pwm is inverted, set to 1 to disable backlight */
>> +		gpio_direction_output(pin, 1);
>> +	}
>> +
>> +	/* Give the backlight some time to turn off and power up the panel. */
>> +	mdelay(40);
>> +	pin = sunxi_name_to_gpio(CONFIG_VIDEO_LCD_POWER);
>> +	if (pin != -1) {
>> +		gpio_request(pin, "lcd_power");
>> +		gpio_direction_output(pin, 1);
>> +	}
>> +}
>> +
>> +static void sunxi_lcdc_backlight_enable(void)
>> +{
>> +	int pin;
>> +
>> +	/*
>> +	 * We want to have scanned out atleast one frame before enabling the
>
> "at least".

Fixed for v2.

>> +	 * backlight to avoid the screen flashing to white when we enable it.
>> +	 */
>> +	mdelay(40);
>
> I take it there isn't a vsync status bit or something we can watch for?

There is a line interrupt, which we could theoretically use, but the docs are
rather lacking, so I believe this is the best solution for now.

>
>> [...]
>> +	switch (sunxi_display.monitor) {
>> +	case sunxi_monitor_none:
>> +		return 0;
>> +	case sunxi_monitor_dvi:
>> +	case sunxi_monitor_hdmi:
>> +		pipeline = "de_be0-lcd0-hdmi";
>> +		break;
>> +	case sunxi_monitor_lcd:
>> +		pipeline = "de_be0-lcd0";
>> +		break;
>> +	case sunxi_monitor_vga:
>> +		break;
>> +	}
>> +
>>   	/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
>
> This comment is no longer accurate.

Fixed for v2.

>>   	offset = fdt_node_offset_by_compatible(blob, -1,
>>   					       "allwinner,simple-framebuffer");
>>   	while (offset >= 0) {
>>   		ret = fdt_find_string(blob, offset, "allwinner,pipeline",
>> -				      "de_be0-lcd0-hdmi");
>> +				      pipeline);
>>   		if (ret == 0)
>>   			break;
>>   		offset = fdt_node_offset_by_compatible(blob, offset,
>
> Ian.

Regards,

Hans