Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

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

 



Hi Magnus,
On 2011/02/24 8:28, Magnus Damm wrote:
Hi Damian,

On Wed, Feb 23, 2011 at 8:22 PM, Damian<dhobsong@xxxxxxxxxx>  wrote:

diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax<dhobsong@xxxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV      0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h
b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
        struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
        struct sh_mobile_lcdc_board_cfg board_cfg;
        struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn
I/F */
+       int nonstd;
  };

  struct sh_mobile_lcdc_info {
--
1.7.1

Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?

My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.

Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?

I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.

Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.


Ok, I see what you're saying. But if the SH_FB_YUV flag is disappearing, I guess it makes sense to ditch the second patch in this series as well, since that's just further abstraction (albeit locally).

Thanks,

/ magnus

Thank you,
--
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux