On 25/09/13 12:00, Dr. H. Nikolaus Schaller wrote: >> 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. I'm not sure I follow. Linux device drivers are drivers for specific pieces of hardware. If the hardware needs to be controlled somehow, like setting gpios, enabling regulators, then we need a driver for that piece of hardware. > And, it must not be the OPA362 - it could be something completely different. Right. If it's something completely different, then you need a driver for the completely different piece of hardware. At the board code, or board's device tree data, you specify which pieces of hardware you have on your board, and the respective drivers are then used. >> 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. Sure, but the gpio is not related to the connector, it's related to another component. And say, if we added the enable gpio to the connector. What if the next board uses another version of OPA that requires two gpios. Should we then add another gpio to the connector? Maybe third also? How about a bunch of regulators? Where does it end? >> 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). Right, I only suggested pinmuxing as that's why we need the callback for DSI. I don't remember the details about DEVCONF1 and tv-out. And I disagree that it's a simple problem =). Making generic, reusable drivers is not simple. Making a hack to make it work on your particular platform and board is simple, though. > 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? No, those are properties of the SoC, as far as I understand, nothing to do with the connector. Although I have to say I have no idea what "TV AC coupled load enable" or "Active high enable AVDAC TV out bypass" do. I guess I need to refresh my memory on the VENC, although if I recall right, it wasn't explained very well in the TRM. Also, note that drivers cannot touch DEVCONF1 register. That can only be touched by the OMAP's platform code. So there has to be some kind of callback or API to do that. And while the display drivers for OMAP are currently OMAP specific, they will be made generic at some point, and actually are already designed to be so. So they may not contain any OMAP'isms. > I think this would be very similar to the "invert_polarity" property. I have to say that I don't quite remember what the invert_polarity is used for, but I think invert_polarity might be misplaced also. Is the analog video signal that goes out from the connector inverted? I don't think so. Does the connector require an inverted signal? No, as the connector is really nothing else than a pass-though connector. Correct me if I'm wrong, but I think the invert_polarity is required if there's something on the board that requires the signal to be inverted (OPA?), and it'll be re-inverterted before the signal goes to the analog connector. > 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. Well, I was never too well into the analog TV-out side, so please argue if I say something silly here. But generally speaking, there are two different kinds of properties here. Consider DPI output from the SoC, and a DPI panel. Let's say the DPI panel requires horizontal sync signal to be active high. It's a property of the panel, and it makes sense (at least to me) to think that the DPI panel asks the DPI encoder to provide a signal with hsync active high. Also, it doesn't matter what kind of DPI encoder there is, the hsync active high is a "universal" property. Then, let's say the board has some electrical issues and requires the DPI signals to be of slightly higher voltage than normally. I think OMAP at least has bits to driver some pins with higher voltage. This property is not about panel. There could be lots of different kinds of tweaks for different SoCs, and they are SoC (or DPI encoder) specific things. Here is makes no sense to have a property for the panel. To me, enable_ac and enable_bypass sound like the latter kind. Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
