Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-04 Thread Philipp Zabel
Hi Russell,

Am Mittwoch, den 03.12.2014, 16:30 + schrieb Russell King - ARM
Linux:
 On Wed, Dec 03, 2014 at 05:20:15PM +0100, Philipp Zabel wrote:
  Hi Andy,
  
  It would be better if the bind function would not have to care about
  platform resources, that should be handled in the probe function. I had
  a patch to move them:
  
  http://lists.freedesktop.org/archives/dri-devel/2014-May/059630.html
  
  Maybe you could incorporate something like this?
 
 Personally, I hate this idea.  Having a two-layered setup means that
 the when the bind() method is called, the state of struct imx_hdmi is
 indeterminant.
 
 If it's called immediately from probe, most of the structure will be
 zeroed, and only those members initialised by the probe function will
 be set to non-zero values.
 
 However, if the HDMI interface has been previously bound, and is
 subsequently re-bound, then the structure will most definitely no
 longer be in a known state on the second bind() call.
 
 This is fragile.
 
 Now, people have tried to tell me that this isn't fragile, but, I now
 have proof that it is as fragile as I fear.  The component helper
 doesn't yet have that many users, and already we have one user (okay,
 it's not part of the mainline kernel - it's etnaviv) which contained
 exactly this kind of bug: it expected its private structures to be
 zeroed on the bind() call.

 So, I /really/ hate this idea.  If you really want to do this, then
 please ensure that the bind() call explicitly zeros the bits of the
 struct which aren't initialised by the probe() call, so we know that
 the driver will always start off with a known initial state.

You are right, no I don't want this. When I initially wrote this patch I
was under the impression that the memory allocated by devm_kzalloc in
bind() wouldn't be freed on unbind(). I remember I stopped pursuing this
patch when I got aware of the devres_open/close/remove_group dance in
the component framework code, but somehow forgot to drop it altogether
locally.

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-04 Thread Russell King - ARM Linux
On Thu, Dec 04, 2014 at 09:40:10AM +0100, Philipp Zabel wrote:
 You are right, no I don't want this. When I initially wrote this patch I
 was under the impression that the memory allocated by devm_kzalloc in
 bind() wouldn't be freed on unbind().

Resources claimed inside bind() will be freed in unbind().  Resources
claimed in the driver's probe() will be freed in driver's remove().

They nest, and each is properly dealt with at the appropriate time due
to...

 I remember I stopped pursuing this
 patch when I got aware of the devres_open/close/remove_group dance in
 the component framework code, but somehow forgot to drop it altogether
 locally.

... the use of devres grouping.

So, if you use devm_kzalloc() in the driver's probe() function, then
that memory will be freed after the driver's remove() function is
called.  That's fine.

The point I was making is:

probe()
  mem = devm_kzalloc();
  mem-mmio = ...;
...
bind()
  mem-mmio is set
  other members of mem are zero
...
unbind()
...
bind()
  mem-mmio is set
  other members of mem are indeterminant
...
unbind()
...
remove()
  mem will be freed automatically

That's where the problem happens - the second time the bind() function
gets called: you might as well not use devm_k*z*alloc() initially,
because having the structure initialised to zero _could_ very well
hide bugs.

When you consider that it's not just the driver code which you have
to audit, but also any code the driver calls (eg, because you've embedded
some subsystem's struct in your driver private data) it quickly becomes
very easy for a bug to creep in here.

If we want to go down the route of having the probe function deal with
resources etc, then I would recommend against using devm_kzalloc() to
allocate the private structure: use devm_kmalloc() instead, which will
leave the memory uninitialised.  That means you get the same condition
on each bind(), which means you have to think more about how to ensure
that the initial state of members is dealt with throughout your driver.

Alternatively, separate the struct into two sections: one which contains
everything initialised by the probe() function, and everything else, and
arrange for everything else to get memset() on entry to bind().

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Andy Yan
IMX6 and Rockchip RK3288 and JZ4780 (Ingenic Xburst/MIPS)
use the interface compatible Designware HDMI IP, but they
also have some lightly differences, such as phy pll configuration,
register width, 4K support, clk useage, and the crtc mux configuration
is also platform specific.

To reuse the imx hdmi driver, convert it to drm_bridge

handle encoder in imx-hdmi_pltfm.c, as most of the encoder
operation are platform specific such as crtc select and
panel format set

This patch depends on Russell King's patch:
 drm: imx: convert imx-drm to use the generic DRM OF helper
 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-July/053484.html

Signed-off-by: Andy Yan andy@rock-chips.com
Signed-off-by: Yakir Yang y...@rock-chips.com

---

Changes in v16:
- use the common binding for the clocks

Changes in v15: None
Changes in v14:
- add defer probing, adviced by Philipp Zabel

Changes in v13:
- split platform specific phy configuration

Changes in v12:
- squash patch convert dw_hdmi to drm_bridge

Changes in v11:
- squash patch  split some phy configuration to platform driver

Changes in v10:
- split generic dw_hdmi.c improvements from patch#11 (add rk3288 support)

Changes in v9: None
Changes in v8: None
Changes in v7:
- remove unused variables from structure dw_hdmi
- remove a wrong modification
- add copyrights for dw_hdmi-imx.c

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None

 drivers/gpu/drm/imx/Makefile |   2 +-
 drivers/gpu/drm/imx/imx-hdmi.c   | 252 +--
 drivers/gpu/drm/imx/imx-hdmi.h   |  14 ++
 drivers/gpu/drm/imx/imx-hdmi_pltfm.c | 193 +++
 4 files changed, 304 insertions(+), 157 deletions(-)
 create mode 100644 drivers/gpu/drm/imx/imx-hdmi_pltfm.c

diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
index 582c438..63cf56a 100644
--- a/drivers/gpu/drm/imx/Makefile
+++ b/drivers/gpu/drm/imx/Makefile
@@ -9,4 +9,4 @@ obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
 
 imx-ipuv3-crtc-objs  := ipuv3-crtc.o ipuv3-plane.o
 obj-$(CONFIG_DRM_IMX_IPUV3)+= imx-ipuv3-crtc.o
-obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o
+obj-$(CONFIG_DRM_IMX_HDMI) += imx-hdmi.o imx-hdmi_pltfm.o
diff --git a/drivers/gpu/drm/imx/imx-hdmi.c b/drivers/gpu/drm/imx/imx-hdmi.c
index 7a54d20..a7c1ec7 100644
--- a/drivers/gpu/drm/imx/imx-hdmi.c
+++ b/drivers/gpu/drm/imx/imx-hdmi.c
@@ -12,25 +12,20 @@
  * Copyright (C) 2010, Guennadi Liakhovetski g.liakhovet...@gmx.de
  */
 
-#include linux/component.h
 #include linux/irq.h
 #include linux/delay.h
 #include linux/err.h
 #include linux/clk.h
 #include linux/hdmi.h
-#include linux/regmap.h
-#include linux/mfd/syscon.h
-#include linux/mfd/syscon/imx6q-iomuxc-gpr.h
 #include linux/of_device.h
 
+#include drm/drm_of.h
 #include drm/drmP.h
 #include drm/drm_crtc_helper.h
 #include drm/drm_edid.h
 #include drm/drm_encoder_slave.h
-#include video/imx-ipu-v3.h
 
 #include imx-hdmi.h
-#include imx-drm.h
 
 #define HDMI_EDID_LEN  512
 
@@ -54,11 +49,6 @@ enum hdmi_datamap {
YCbCr422_12B = 0x12,
 };
 
-enum imx_hdmi_devtype {
-   IMX6Q_HDMI,
-   IMX6DL_HDMI,
-};
-
 static const u16 csc_coeff_default[3][4] = {
{ 0x2000, 0x, 0x, 0x },
{ 0x, 0x2000, 0x, 0x },
@@ -113,7 +103,8 @@ struct hdmi_data_info {
 
 struct imx_hdmi {
struct drm_connector connector;
-   struct drm_encoder encoder;
+   struct drm_encoder *encoder;
+   struct drm_bridge *bridge;
 
enum imx_hdmi_devtype dev_type;
struct device *dev;
@@ -121,6 +112,7 @@ struct imx_hdmi {
struct clk *iahb_clk;
 
struct hdmi_data_info hdmi_data;
+   const struct imx_hdmi_plat_data *plat_data;
int vic;
 
u8 edid[HDMI_EDID_LEN];
@@ -137,13 +129,6 @@ struct imx_hdmi {
int ratio;
 };
 
-static void imx_hdmi_set_ipu_di_mux(struct imx_hdmi *hdmi, int ipu_di)
-{
-   regmap_update_bits(hdmi-regmap, IOMUXC_GPR3,
-  IMX6Q_GPR3_HDMI_MUX_CTL_MASK,
-  ipu_di  IMX6Q_GPR3_HDMI_MUX_CTL_SHIFT);
-}
-
 static inline void hdmi_writeb(struct imx_hdmi *hdmi, u8 val, int offset)
 {
writeb(val, hdmi-regs + offset);
@@ -1371,6 +1356,50 @@ static void imx_hdmi_poweroff(struct imx_hdmi *hdmi)
imx_hdmi_phy_disable(hdmi);
 }
 
+static void imx_hdmi_bridge_mode_set(struct drm_bridge *bridge,
+struct drm_display_mode *mode,
+struct drm_display_mode *adjusted_mode)
+{
+   struct imx_hdmi *hdmi = bridge-driver_private;
+
+   imx_hdmi_setup(hdmi, mode);
+
+   /* Store the display mode for plugin/DKMS poweron events */
+   memcpy(hdmi-previous_mode, mode, sizeof(hdmi-previous_mode));
+}
+
+static bool imx_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
+  const struct drm_display_mode *mode,
+  

Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Russell King - ARM Linux
On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:
 +int imx_hdmi_bind(struct device *dev, struct device *master,
 +   void *data, struct drm_encoder *encoder,
 +   const struct imx_hdmi_plat_data *plat_data)
  {
   struct platform_device *pdev = to_platform_device(dev);
 - const struct of_device_id *of_id =
 - of_match_device(imx_hdmi_dt_ids, dev);
   struct drm_device *drm = data;
   struct device_node *np = dev-of_node;
   struct device_node *ddc_node;
 @@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct 
 device *master, void *data)
   struct resource *iores;
   int ret, irq;
  
 - hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
 + hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
   if (!hdmi)
   return -ENOMEM;
  
 - hdmi-dev = dev;
 + hdmi-plat_data = plat_data;
 + hdmi-dev = pdev-dev;
 + hdmi-dev_type = plat_data-dev_type;
   hdmi-sample_rate = 48000;
   hdmi-ratio = 100;
 -
 - if (of_id) {
 - const struct platform_device_id *device_id = of_id-data;
 -
 - hdmi-dev_type = device_id-driver_data;
 - }
 + hdmi-encoder = encoder;

I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
number, and avoiding the platform device stuff altogether in here.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Andy Yan

Hi Russell:

On 2014年12月03日 23:38, Russell King - ARM Linux wrote:

On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:

+int imx_hdmi_bind(struct device *dev, struct device *master,
+ void *data, struct drm_encoder *encoder,
+ const struct imx_hdmi_plat_data *plat_data)
  {
struct platform_device *pdev = to_platform_device(dev);
-   const struct of_device_id *of_id =
-   of_match_device(imx_hdmi_dt_ids, dev);
struct drm_device *drm = data;
struct device_node *np = dev-of_node;
struct device_node *ddc_node;
@@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct 
device *master, void *data)
struct resource *iores;
int ret, irq;
  
-	hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);

+   hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
  
-	hdmi-dev = dev;

+   hdmi-plat_data = plat_data;
+   hdmi-dev = pdev-dev;
+   hdmi-dev_type = plat_data-dev_type;
hdmi-sample_rate = 48000;
hdmi-ratio = 100;
-
-   if (of_id) {
-   const struct platform_device_id *device_id = of_id-data;
-
-   hdmi-dev_type = device_id-driver_data;
-   }
+   hdmi-encoder = encoder;

I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
number, and avoiding the platform device stuff altogether in here.


   Actually this is what the current code do: the resource and irq number
are all handled in imx_hdmi_bind


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Russell King - ARM Linux
On Thu, Dec 04, 2014 at 12:04:37AM +0800, Andy Yan wrote:
 Hi Russell:
 
 On 2014年12月03日 23:38, Russell King - ARM Linux wrote:
 On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:
 +int imx_hdmi_bind(struct device *dev, struct device *master,
 + void *data, struct drm_encoder *encoder,
 + const struct imx_hdmi_plat_data *plat_data)
   {
 struct platform_device *pdev = to_platform_device(dev);
 -   const struct of_device_id *of_id =
 -   of_match_device(imx_hdmi_dt_ids, dev);
 struct drm_device *drm = data;
 struct device_node *np = dev-of_node;
 struct device_node *ddc_node;
 @@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct 
 device *master, void *data)
 struct resource *iores;
 int ret, irq;
 -   hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
 +   hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
 if (!hdmi)
 return -ENOMEM;
 -   hdmi-dev = dev;
 +   hdmi-plat_data = plat_data;
 +   hdmi-dev = pdev-dev;
 +   hdmi-dev_type = plat_data-dev_type;
 hdmi-sample_rate = 48000;
 hdmi-ratio = 100;
 -
 -   if (of_id) {
 -   const struct platform_device_id *device_id = of_id-data;
 -
 -   hdmi-dev_type = device_id-driver_data;
 -   }
 +   hdmi-encoder = encoder;
 I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
 number, and avoiding the platform device stuff altogether in here.
 
Actually this is what the current code do: the resource and irq number
 are all handled in imx_hdmi_bind

I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Philipp Zabel
Hi Andy,

Am Donnerstag, den 04.12.2014, 00:04 +0800 schrieb Andy Yan:
 On 2014年12月03日 23:38, Russell King - ARM Linux wrote:
  On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:
  +int imx_hdmi_bind(struct device *dev, struct device *master,
  +void *data, struct drm_encoder *encoder,
  +const struct imx_hdmi_plat_data *plat_data)
{
 struct platform_device *pdev = to_platform_device(dev);
  -  const struct of_device_id *of_id =
  -  of_match_device(imx_hdmi_dt_ids, dev);
 struct drm_device *drm = data;
 struct device_node *np = dev-of_node;
 struct device_node *ddc_node;
  @@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, 
  struct device *master, void *data)
 struct resource *iores;
 int ret, irq;

  -  hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
  +  hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
 if (!hdmi)
 return -ENOMEM;

  -  hdmi-dev = dev;
  +  hdmi-plat_data = plat_data;
  +  hdmi-dev = pdev-dev;
  +  hdmi-dev_type = plat_data-dev_type;
 hdmi-sample_rate = 48000;
 hdmi-ratio = 100;
  -
  -  if (of_id) {
  -  const struct platform_device_id *device_id = of_id-data;
  -
  -  hdmi-dev_type = device_id-driver_data;
  -  }
  +  hdmi-encoder = encoder;
  I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
  number, and avoiding the platform device stuff altogether in here.
 
 Actually this is what the current code do: the resource and irq number
 are all handled in imx_hdmi_bind

It would be better if the bind function would not have to care about
platform resources, that should be handled in the probe function. I had
a patch to move them:

http://lists.freedesktop.org/archives/dri-devel/2014-May/059630.html

Maybe you could incorporate something like this?

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Andy Yan


On 2014年12月04日 00:11, Russell King - ARM Linux wrote:

On Thu, Dec 04, 2014 at 12:04:37AM +0800, Andy Yan wrote:

Hi Russell:

On 2014年12月03日 23:38, Russell King - ARM Linux wrote:

On Wed, Dec 03, 2014 at 11:29:26PM +0800, Andy Yan wrote:

+int imx_hdmi_bind(struct device *dev, struct device *master,
+ void *data, struct drm_encoder *encoder,
+ const struct imx_hdmi_plat_data *plat_data)
  {
struct platform_device *pdev = to_platform_device(dev);
-   const struct of_device_id *of_id =
-   of_match_device(imx_hdmi_dt_ids, dev);
struct drm_device *drm = data;
struct device_node *np = dev-of_node;
struct device_node *ddc_node;
@@ -1594,19 +1566,16 @@ static int imx_hdmi_bind(struct device *dev, struct 
device *master, void *data)
struct resource *iores;
int ret, irq;
-   hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
+   hdmi = devm_kzalloc(pdev-dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
-   hdmi-dev = dev;
+   hdmi-plat_data = plat_data;
+   hdmi-dev = pdev-dev;
+   hdmi-dev_type = plat_data-dev_type;
hdmi-sample_rate = 48000;
hdmi-ratio = 100;
-
-   if (of_id) {
-   const struct platform_device_id *device_id = of_id-data;
-
-   hdmi-dev_type = device_id-driver_data;
-   }
+   hdmi-encoder = encoder;

I'd suggest changing imx_hdmi_bind() to take the struct resource and irq
number, and avoiding the platform device stuff altogether in here.


Actually this is what the current code do: the resource and irq number
are all handled in imx_hdmi_bind

I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.

   if so, how about the device tree properties  ddc-i2c-bus, 
reg-io-width, iahb, isfr,

  they are all found by device?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Russell King - ARM Linux
On Wed, Dec 03, 2014 at 05:20:15PM +0100, Philipp Zabel wrote:
 Hi Andy,
 
 It would be better if the bind function would not have to care about
 platform resources, that should be handled in the probe function. I had
 a patch to move them:
 
 http://lists.freedesktop.org/archives/dri-devel/2014-May/059630.html
 
 Maybe you could incorporate something like this?

Personally, I hate this idea.  Having a two-layered setup means that
the when the bind() method is called, the state of struct imx_hdmi is
indeterminant.

If it's called immediately from probe, most of the structure will be
zeroed, and only those members initialised by the probe function will
be set to non-zero values.

However, if the HDMI interface has been previously bound, and is
subsequently re-bound, then the structure will most definitely no
longer be in a known state on the second bind() call.

This is fragile.

Now, people have tried to tell me that this isn't fragile, but, I now
have proof that it is as fragile as I fear.  The component helper
doesn't yet have that many users, and already we have one user (okay,
it's not part of the mainline kernel - it's etnaviv) which contained
exactly this kind of bug: it expected its private structures to be
zeroed on the bind() call.

So, I /really/ hate this idea.  If you really want to do this, then
please ensure that the bind() call explicitly zeros the bits of the
struct which aren't initialised by the probe() call, so we know that
the driver will always start off with a known initial state.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Russell King - ARM Linux
On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote:
 
 On 2014年12月04日 00:11, Russell King - ARM Linux wrote:
 I meant that imx_hdmi_bind should be passed these, so that it needs to
 know nothing about the struct device beyond the generic device structure.
 In other words, the dw-hdmi core should not assume that the struct device
 is part of a platform device.
 
if so, how about the device tree properties  ddc-i2c-bus, reg-io-width,
 iahb, isfr,
   they are all found by device?

If the device has a device tree node associated with it, it will have a
non-NULL dev-of_node - which is part of the generic device structure.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Andy Yan

Hi Russell:
On 2014年12月04日 00:33, Russell King - ARM Linux wrote:

On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote:

On 2014年12月04日 00:11, Russell King - ARM Linux wrote:

I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.


if so, how about the device tree properties  ddc-i2c-bus, reg-io-width,
iahb, isfr,
   they are all found by device?

If the device has a device tree node associated with it, it will have a
non-NULL dev-of_node - which is part of the generic device structure.

  so , I just need get the resource and irq number in the 
dw_hdmi-imx/rockchip ,than
  pass them to imx_hdmi_bind, as the properties ddc-i2c-bus, 
reg-io-width, iahb,isfr, they

  are still can be handled in imx_hdmi_bind ?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Russell King - ARM Linux
On Thu, Dec 04, 2014 at 12:56:24AM +0800, Andy Yan wrote:
 Hi Russell:
 On 2014年12月04日 00:33, Russell King - ARM Linux wrote:
 On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote:
 On 2014年12月04日 00:11, Russell King - ARM Linux wrote:
 I meant that imx_hdmi_bind should be passed these, so that it needs to
 know nothing about the struct device beyond the generic device structure.
 In other words, the dw-hdmi core should not assume that the struct device
 is part of a platform device.
 
 if so, how about the device tree properties  ddc-i2c-bus, reg-io-width,
 iahb, isfr,
they are all found by device?
 If the device has a device tree node associated with it, it will have a
 non-NULL dev-of_node - which is part of the generic device structure.
 
   so , I just need get the resource and irq number in the
 dw_hdmi-imx/rockchip ,than
   pass them to imx_hdmi_bind, as the properties ddc-i2c-bus, reg-io-width,
 iahb,isfr, they
   are still can be handled in imx_hdmi_bind ?

Basically, what I'm suggesting is just this change to imx_hdmi_bind():

 int imx_hdmi_bind(struct device *dev, struct device *master,
  void *data, struct drm_encoder *encoder,
+ const struct resource *iores, int irq,
  const struct imx_hdmi_plat_data *plat_data)
 {
-   struct platform_device *pdev = to_platform_device(dev);
...
}

-   irq = platform_get_irq(pdev, 0);
if (irq  0)
return irq;
...
return ret;

-   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hdmi-regs = devm_ioremap_resource(dev, iores);
if (IS_ERR(hdmi-regs))

and supplying those as arguments from the caller.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v16 03/12] drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode

2014-12-03 Thread Andy Yan

Hi Russel:
On 2014年12月04日 07:40, Russell King - ARM Linux wrote:

On Thu, Dec 04, 2014 at 12:56:24AM +0800, Andy Yan wrote:

Hi Russell:
On 2014年12月04日 00:33, Russell King - ARM Linux wrote:

On Thu, Dec 04, 2014 at 12:30:23AM +0800, Andy Yan wrote:

On 2014年12月04日 00:11, Russell King - ARM Linux wrote:

I meant that imx_hdmi_bind should be passed these, so that it needs to
know nothing about the struct device beyond the generic device structure.
In other words, the dw-hdmi core should not assume that the struct device
is part of a platform device.


if so, how about the device tree properties  ddc-i2c-bus, reg-io-width,
iahb, isfr,
   they are all found by device?

If the device has a device tree node associated with it, it will have a
non-NULL dev-of_node - which is part of the generic device structure.


   so , I just need get the resource and irq number in the
dw_hdmi-imx/rockchip ,than
   pass them to imx_hdmi_bind, as the properties ddc-i2c-bus, reg-io-width,
iahb,isfr, they
   are still can be handled in imx_hdmi_bind ?

Basically, what I'm suggesting is just this change to imx_hdmi_bind():

  int imx_hdmi_bind(struct device *dev, struct device *master,
  void *data, struct drm_encoder *encoder,
+ const struct resource *iores, int irq,
  const struct imx_hdmi_plat_data *plat_data)
  {
-   struct platform_device *pdev = to_platform_device(dev);
...
}

-   irq = platform_get_irq(pdev, 0);
if (irq  0)
return irq;
...
return ret;

-   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hdmi-regs = devm_ioremap_resource(dev, iores);
if (IS_ERR(hdmi-regs))

and supplying those as arguments from the caller.


  got it, thanks, and also many thanks for Philipp


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel