Re: DSS display-new custom enable/disable hooks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tomi,

Am 25.09.2013 um 10:29 schrieb Tomi Valkeinen:

> On 25/09/13 10:45, Dr. H. Nikolaus Schaller wrote:
>> Hi,
>> 
>> Am 25.09.2013 um 09:09 schrieb Belisko Marek:
>> 
>>> CC - Nikolaus Schaller
>>> 
>>> On Wed, Sep 25, 2013 at 8:12 AM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
>>>> On 24/09/13 23:04, Belisko Marek wrote:
>>>>> Hi,
>>>>> 
>>>>> we're using connector-analog-tv driver to enable TV out on gta04
>>>>> board. There is exception that we need to change some twl registers +
>>>>> some gpio when enable/disable TV output. My question is if there is
>>>>> some way how to do that or do we need to copy'n'paste code from
>>>>> connector-analog-tv driver and extend it for handling we need (let's
>>>>> call it hack)?
>>>> 
>>>> What are those TWL registers and GPIO used for?
>> 
>> The GTA04 devices uses an OPA362 video amplifier connected to the TV_VFB1 pin
>> of the DM3730. According to some not well known discussion with TI [1] we must
>> therefore configure the AC bias and Bypass through the DEVCONF1 register.
> 
> Ah, OPA. A blast from the past =).
> 
> Well, OPA is a distinct hardware block in the video path, I see no issue
> in having an OPA encoder driver, that sits in between VENC and the
> connector.
> 
> The driver should handle things described in the OPA datasheet. With a
> quick glance, there seems to be one GPIO and one power line to OPA.

Hm. Why need a driver for a specific piece of hardware? For software it is just an
analog TV out with additional control options.

And, it must not be the OPA362 - it could be something completely different.

> 
>> [1] https://e2e.ti.com/support/dsp/omap_applications_processors/f/447/p/94072/343691.aspx
>> 
>> And we have to disable (High-Z) the OPA362 amplifier if TVout is not in use since the
>> line is shared with a different function (microphone input).
>> 
>> The reasons to use the OPA362 at all are to do short circuit protection and to high-Z
>> the driver (both are n/a with the DM3730 as far as we know).
>> 
>> The code we did use successfully in the 2.6.32 board file is:
>> 
>> 120 #define TV_OUT_GPIO     23
>> 
>> 374 static int gta04_panel_enable_tv(struct omap_dss_device *dssdev)
>> 375 {
>> 376         u32 reg;
>> 377 
>> 378 #define ENABLE_VDAC_DEDICATED           0x03
>> 379 #define ENABLE_VDAC_DEV_GRP             0x20
>> 380 #define OMAP2_TVACEN                            (1 << 11)
>> 381 #define OMAP2_TVOUTBYPASS                       (1 << 18)
>> 382 
>> 383         twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER,
>> 384                         ENABLE_VDAC_DEDICATED,
>> 385                         TWL4030_VDAC_DEDICATED);
>> 386         twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER,
>> 387                         ENABLE_VDAC_DEV_GRP, TWL4030_VDAC_DEV_GRP);
> 
> All that i2c stuff should be handled by using the normal regulator
> framework.
> 
>> 388 
>> 389         /* idea taken from https://e2e.ti.com/support/dsp/omap_applications_processors/f/447/p/94072/343691.aspx */
>> 390         reg = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> 391 //      printk(KERN_INFO "Value of DEVCONF1 was: %08x\n", reg);
>> 392         reg |= OMAP2_TVOUTBYPASS;       /* enable TV bypass mode for external video driver (for OPA362 driver) */
>> 393         reg |= OMAP2_TVACEN;            /* assume AC coupling to remove DC offset */
>> 394         omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
>> 395         reg = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> 396 //      printk(KERN_INFO "Value of DEVCONF1 now: %08x\n", reg);
>> 397 
>> 398         gpio_set_value(TV_OUT_GPIO, 1); /* enable output driver (OPA362) */
>> 399 
>> 400         return 0;
>> 401 }
>> 402 
>> 403 static void gta04_panel_disable_tv(struct omap_dss_device *dssdev)
>> 404 {
>> 405         gpio_set_value(TV_OUT_GPIO, 0); /* disable output driver (and re-enable microphone) */
>> 406 
>> 407         twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0x00,
>> 408                         TWL4030_VDAC_DEDICATED);
>> 409         twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0x00,
>> 410                         TWL4030_VDAC_DEV_GRP);
>> 411 }
>> 412
>> 413 static struct omap_dss_device gta04_tv_device = {
>> 414         .name = "tv",
>> 415         .driver_name = "venc",
>> 416         .type = OMAP_DISPLAY_TYPE_VENC,
>> 417         /* GTA04 has a single composite output (with external video driver) */
>> 418         .phy.venc.type = OMAP_DSS_VENC_TYPE_COMPOSITE,
>> 419         .phy.venc.invert_polarity = true,       /* needed if we use external video driver */
>> 420         .platform_enable = gta04_panel_enable_tv,
>> 421         .platform_disable = gta04_panel_disable_tv,
>> 422 };
>> 
>> I think (but don't know) that controlling the TWL/VDAC is no longer needed because
>> it should be done through the connector-analog-tv driver.
> 
> I don't think VDAC is related to the connector (although I didn't
> actually look at the analog video when I created the new
> connector-analog-driver). If there's something specifically related to
> an analog tv out connector that requires power, then, yes, it's needed.
> But if it's all about OMAP's VENC and OPA, then the connector itself
> does not require anything.
> 
> As an example, DVI connector does require a +5V (if I recall right), and
> that +5V is not related to the DVI video source, it's only about the
> connector. So in that case, the DVI connector driver should handle the
> +5V (it doesn't, currently, but will soon be added).
> 
>> So we need to
>> 1. control a GPIO - very similar to a TFT panel
>> 2. invert polarity
>> 3. configure the OMAP343X_CONTROL_DEVCONF1 register as needed
>> 
>> I don't know if we must do step 3. each time we enable TVout (because we can't
>> assume that no other part of the kernel code writes to the DEVCONF1 register) so that
>> it could be done in the board init code.
> 
> So, is the GPIO the one that goes to OPA?

Yes. Like the enable_gpio in struct panel_dpi_platform_data.

So I now think we would be happy if the connector-analog-tv would have a gpio
in its platform data that can handle such an enable/disable.

> Is VDAC the power to OPA?

No. The OPA is powered by an external 3.3V rail. I don't really remember why
we control VDAC in that piece of code. It was copied from the beagle board
validation kernel. Maybe it was not done automatically by the 2.6.32 DSS code.

> VDAC
> is already used by VENC, which manages it, so if VDAC is only used for
> VENC, there's no need for to handle it anywhere else.

That is what I also would assume.

> 
> The DEVCONF1 register is problematic. For that we need some kind of
> platform hooks. We have some already for PM and DSI pins, so we could
> add a new one for TV-out. However, the DSI pins (hopefully) could be
> handled by the pinmuxing framework. Maybe that would be applicable here
> also.

Hm. Looks like a complex solution for a simple problem and isn't related to
pinmuxing (IMHO).

Could we just add a "enable_ac" and "enable_bypass"  to struct connector_atv_platform_data,
that sets/resets these flags in the DEVCONF1 register each time the tvout is enabled?

I think this would be very similar to the "invert_polarity" property.

Then, the platform data has more control over the shape of the signals created by
the VENC (independently of that we use an OPA362).

IMHO this topic is very similar to controlling the polarity and timing of the
HSYNC/VSYNC of a LCD (.flags in struct display_timing), i.e. shaping the
interface signals so that they are accepted by the hardware connected to
the DM3730.

If you think this would be a reasonable approach, we can try to prepare a
patch to connector-analog-tv.c for further discussion.

BR,
Nikolaus

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux