Hi Tomi,
Thanks for reviewing!
On 10/22/2012 02:40 AM, Tomi Valkeinen wrote:
On 2012-10-16 04:27, Ricardo Neri wrote:
Creating the accessory devices, such as audio, from the HDMI driver
allows to regard HDMI as a single entity with audio an display
functionality. This intends to follow the design of drivers such
as MFD, in which a single entity handles the creation of the accesory
devices. Such devices are then used by domain-specific drivers; audio in
this case.
Also, this is in line with the DT implementation of HDMI, in which we will
have a single node to describe this feature of the OMAP SoC.
Signed-off-by: Ricardo Neri <ricardo.neri@xxxxxx>
---
drivers/video/omap2/dss/hdmi.c | 68 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index e5be0a5..c62c5ab 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -60,6 +60,9 @@
static struct {
struct mutex lock;
struct platform_device *pdev;
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+ struct platform_device *audio_pdev;
+#endif
struct hdmi_ip_data ip_data;
@@ -73,6 +76,13 @@ static struct {
struct omap_dss_output output;
} hdmi;
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+#define HDMI_AUDIO_MEM_RESOURCE 0
+#define HDMI_AUDIO_DMA_RESOURCE 1
I don't see much point with these definitions. They are hdmi.c internal,
so the audio driver can't use them, and so they aren't really fixed.
I just thought it could make the code more readable; but if the
resources array is going to be local, then they are not helpful.
+static struct resource hdmi_aud_res[2];
Did you check if the platform_device_register does a copy of these? If
it does, this can be local to the probe function.
Thanks! I checked, platform_device_register does the copy. I will put it
as a local variable.
+#endif
+
+
/*
* Logic for the below structure :
* user enters the CEA or VESA timings by specifying the HDMI/DVI code.
@@ -765,6 +775,50 @@ static void hdmi_put_clocks(void)
}
#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+static int hdmi_probe_audio(struct platform_device *pdev)
+{
+ struct resource *res;
+
+ hdmi.audio_pdev = ERR_PTR(-EINVAL);
+
+ res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ DSSERR("can't get IORESOURCE_MEM HDMI\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Pass this resource to audio_pdev.
+ * Audio drivers should not remap it
+ */
+ hdmi_aud_res[HDMI_AUDIO_MEM_RESOURCE].start = res->start;
+ hdmi_aud_res[HDMI_AUDIO_MEM_RESOURCE].end = res->end;
+ hdmi_aud_res[HDMI_AUDIO_MEM_RESOURCE].flags = IORESOURCE_MEM;
+
+ res = platform_get_resource(hdmi.pdev, IORESOURCE_DMA, 0);
+ if (!res) {
+ DSSERR("can't get IORESOURCE_DMA HDMI\n");
+ return -EINVAL;
+ }
+
+ /* Pass this resource to audio_pdev */
+ hdmi_aud_res[HDMI_AUDIO_DMA_RESOURCE].start = res->start;
+ hdmi_aud_res[HDMI_AUDIO_DMA_RESOURCE].end = res->end;
+ hdmi_aud_res[HDMI_AUDIO_DMA_RESOURCE].flags = IORESOURCE_DMA;
+
+ /* create platform device for HDMI audio driver */
+ hdmi.audio_pdev = platform_device_register_simple(
+ "omap_hdmi_audio",
+ -1, hdmi_aud_res,
+ ARRAY_SIZE(hdmi_aud_res));
+ if (IS_ERR(hdmi.audio_pdev)) {
+ DSSERR("Can't instantiate hdmi-audio\n");
+ return PTR_ERR(hdmi.audio_pdev);
+ }
+
+ return 0;
+}
So, how will this work? All the audio related functions will be removed
from the (video) hdmi driver, and the audio driver will access the
registers independently? The audio driver will still need to access the
video parts, right?
That could be a new approach, but the idea here is to continue having an
omapdss audio interface for audio drivers to use.
The root problem that I am trying to address is that the omapdss audio
interface does not have functionality for DMA transfer of audio samples
to the HDMI IP. Also, I am not sure how that could be done without
duplicating the functionality that ASoC already provides.
I feel a bit uneasy about giving the same ioremapped register space to
two independent drivers... If we could split the registers to video and
audio parts, each driver only ioremapping their respective registers,
it'd be much better.
Fwiw, the audio drivers (at least my audio drivers) will not ioremap.
They will just take the DMA request number and port. Maybe spliting the
register space into audio and video is not practical as we would endup
having many tiny address spaces.
BR,
Ricardo
Tomi
--
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
