Re: [PATCH 4/5] OMAPDSS: Export functions to enable dynamic linking

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

 



On Tue, 2012-02-14 at 18:00 -0600, Ricardo Neri wrote:
> Hi Tomi,
> 
> Thanks for your comments.
> On Mon, 2012-02-13 at 15:48 +0200, Tomi Valkeinen wrote:
> > Hi,
> > 
> > On Sat, 2012-02-11 at 17:55 -0600, Ricardo Neri wrote:
> > > The functions dss_init_hdmi_ip_ops and dss_has_feature are used by
> > > the ASoC HDMI codec. Both the ASoC codec and DSS may be built
> > > as separate kernel modules. Hence, these two functions need to be
> > > available for dynamic linking.
> > 
> > Neither of those functions should be exported, they are omapdss
> > internal. And you had to hack around to make those usable by adding
> > drivers/video/omap2/dss into the include path.
> > 
> > Anything that is exported from omapdss should be added to
> > include/video/omapdss.h. Users of omapdss should never include anything
> > from drivers/video/omap2/dss.
> > 
> > But as I said, neither of those functions should be exported, they are
> > clearly omapdss internal functions. For dss_has_feature we probably need
> > a new function to convey the information about the hdmi features that
> > the audio side needs. Or perhaps it can be in the ip_data.
> I will try to create this new functions and add it to
> include/video/omapdss.h and resubmit the patch. The approach that I am
> following is to maintain an hdmi_ip_data structure in the ASoC HDMI
> codec driver to utilize the functions of the HDMI IP library. If the DSS
> features are stored in the ip_data then I would need a DSS function
> (i.e., dss_features_init or similar) to initialize it. This may not be
> right as that is an omapdss internal function.
> 
> > 
> > For dss_init_hdmi_ip_ops I don't see why the audio part should see it.
> > Initializing the ops should clearly be done by omapdss automatically.
> Yes, the DSS HDMI driver already does this for its own struct
> hdmi_ip_data. However, under my approach, the hdmi_ip_data of the ASoC
> HDMI driver needs to be initialized as well.

Ah, I see, you create your own hdmi_ip_data there. I don't think this is
the right direction to go...

I have a feeling that this won't be an easy task.

> Another approach is to not have an hdmi_ip_data in the ASoC HDMI driver
> and reuse the ip_data of the DSS HDMI driver. In that case, however, a
> lot of set/get functions would need to be implemented in DSS to expose
> audio functionality: HDMI mode, deep color configuration, audio
> infoframe config and core and wrapper configuration for audio. An
> example of this is already present in here:
> 
> http://www.spinics.net/lists/linux-omap/msg64479.html
> 
> Being HDMI special as no other display combines audio and video, maybe a
> special header file could be created for HDMI.

We will have the same issue with DisplayPort, so it's not that special.

The HDMI and DSS architecture doesn't currently have a way to do this
easily. I'm not even sure how this should be done.

However, I think the audio driver should be a bit similar conceptually
than, say, omapfb, but for audio instead of video. What I mean is that
the audio driver should find the omap_dss_device/driver that it wants to
use, and then uses the API offered by the device/driver to handle the
audio.

This would mean adding a bunch of functions into omap_dss_driver.
Preferably the functions would be non-HDMI specific, so that DisplayPort
could use them also, or actually any other display type.

The audio driver should iterate the dss devices to find the ones (there
could well be multiple HDMI outputs) that support audio when the driver
starts.

Does the above make sense?

Btw, another thing, not directly related:

I see these in the hdmi driver:

 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
        defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)

If the audio driver is separate, I don't see need for those. In fact, I
think they are wrong in that case. The HDMI driver should have the audio
support for any user, which could in theory be some other driver than
the audio driver you are working on.

If the amount of audio code is big in the hdmi driver (which I guess it
isn't), we could have a hidden kernel config option to enable the HDMI
audio support. And the audio driver would use Kconfig's "select" to
enable it.

 Tomi

Attachment: signature.asc
Description: This is a digitally signed message part


[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