Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-02-03 Thread Thierry Reding 
On Fri, Jan 30, 2015 at 10:37:19AM -0500, Rob Clark wrote:
 On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com wrote:
  Currently, third party bridge drivers(ptn3460) are dependent
  on the corresponding encoder driver init, since bridge driver
  needs a drm_device pointer to finish drm initializations.
  The encoder driver passes the drm_device pointer to the
  bridge driver. Because of this dependency, third party drivers
  like ptn3460 doesn't adhere to the driver model.
 
  In this patch, we reframe the bridge registration framework
  so that bridge initialization is split into 2 steps, and
  bridge registration happens independent of drm flow:
  --Step 1: gather all the bridge settings independent of drm and
add the bridge onto a global list of bridges.
  --Step 2: when the encoder driver is probed, call drm_bridge_attach
for the corresponding bridge so that the bridge receives
drm_device pointer and continues with connector and other
drm initializations.
 
  The old set of bridge helpers are removed, and a set of new helpers
  are added to accomplish the 2 step initialization.
 
  The bridge devices register themselves onto global list of bridges
  when they get probed by calling drm_bridge_add.
 
  The parent encoder driver waits till the bridge is available
  in the lookup table(by calling of_drm_find_bridge) and then
  continues with its initialization.
 
  The encoder driver should also call drm_bridge_attach to pass
  on the drm_device to the bridge object.
 
  drm_bridge_attach inturn calls bridge-funcs-attach so that
  bridge can continue with drm related initializations.
 
 ok, so I probably should have had a closer look at this before it
 landed in drm-next, so if it is too late to revert (and deal w/
 untangling subsequent patches that depend on this) some of these
 issues we'll just have to fix with follow-on patches.
 
 1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
 in drm.tmpl

drm_panel.c is missing kerneldoc as well, though I have a local patch to
add it. If nobody else steps up I'll take this.

 2) as far as I can tell, the new approach to cleaning up bridge
 objects is to just let them leak !?!

With this series bridges are no longer part of the DRM device and it's
the driver that provides them that needs to free them (in -remove()).
It's not a completely perfect solution yet, but with the registry patch
that I proposed a while back all the remaining issues should go away.
Now if I could get anybody to look at that patch...

Thierry


pgpaoaZZoKn_V.pgp
Description: PGP signature

Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-02-02 Thread Ajay kumar 
Hi Rob,

On Fri, Jan 30, 2015 at 9:07 PM, Rob Clark robdcl...@gmail.com wrote:
 On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com wrote:
 Currently, third party bridge drivers(ptn3460) are dependent
 on the corresponding encoder driver init, since bridge driver
 needs a drm_device pointer to finish drm initializations.
 The encoder driver passes the drm_device pointer to the
 bridge driver. Because of this dependency, third party drivers
 like ptn3460 doesn't adhere to the driver model.

 In this patch, we reframe the bridge registration framework
 so that bridge initialization is split into 2 steps, and
 bridge registration happens independent of drm flow:
 --Step 1: gather all the bridge settings independent of drm and
   add the bridge onto a global list of bridges.
 --Step 2: when the encoder driver is probed, call drm_bridge_attach
   for the corresponding bridge so that the bridge receives
   drm_device pointer and continues with connector and other
   drm initializations.

 The old set of bridge helpers are removed, and a set of new helpers
 are added to accomplish the 2 step initialization.

 The bridge devices register themselves onto global list of bridges
 when they get probed by calling drm_bridge_add.

 The parent encoder driver waits till the bridge is available
 in the lookup table(by calling of_drm_find_bridge) and then
 continues with its initialization.

 The encoder driver should also call drm_bridge_attach to pass
 on the drm_device to the bridge object.

 drm_bridge_attach inturn calls bridge-funcs-attach so that
 bridge can continue with drm related initializations.

 ok, so I probably should have had a closer look at this before it
 landed in drm-next, so if it is too late to revert (and deal w/
 untangling subsequent patches that depend on this) some of these
 issues we'll just have to fix with follow-on patches.

 1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
 in drm.tmpl
Ohh, I totally forgot. I will do this. Just point me to some recent
patch which updates docbook.
 2) as far as I can tell, the new approach to cleaning up bridge
 objects is to just let them leak !?!
I just checked. Only MSM hdmi_bridge is leaking, and this is because
it doesn't use devm_kzalloc. All other bridges use devm_kzalloc,
and hence that memory is automatically freed.
For MSM HDMI, we need to find a place to call hdmi_bridge_destroy.

 I'll also need to update the new bridge in the msm edp code..
 although that isn't such a big deal if I knew how this was *supposed*
 to work..
You just need to convert drm_bridge_init to drm_bridge_attach, and
remove destroy callback. Refer this:
http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-nextid=b5217bf4692218d202d3d2cd772864fa1e10be4d

Regards,
Ajay Kumar
 since what is there now at least doesn't look right..

 BR,
 -R



 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Tested-by: Rahul Sharma rahul.sha...@samsung.com
 Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Tested-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Tested-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
 ---
  drivers/gpu/drm/Makefile   |2 +-
  drivers/gpu/drm/bridge/ptn3460.c   |   27 +-
  drivers/gpu/drm/drm_bridge.c   |   91 
 
  drivers/gpu/drm/drm_crtc.c |   67 ---
  drivers/gpu/drm/msm/hdmi/hdmi.c|4 +-
  drivers/gpu/drm/msm/hdmi/hdmi.h|1 +
  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |6 +--
  drivers/gpu/drm/sti/sti_hda.c  |   10 +---
  drivers/gpu/drm/sti/sti_hdmi.c |   10 +---
  include/drm/bridge/ptn3460.h   |8 +++
  include/drm/drm_crtc.h |   26 -
  11 files changed, 133 insertions(+), 119 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_bridge.c

 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index e620807..c83ef2d 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -14,7 +14,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
 drm_info.o drm_debugfs.o drm_encoder_slave.o \
 drm_trace_points.o drm_global.o drm_prime.o \
 drm_rect.o drm_vma_manager.o drm_flip_work.o \
 -   drm_modeset_lock.o drm_atomic.o
 +   drm_modeset_lock.o drm_atomic.o drm_bridge.o

  drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index a2ddc8d..4a818c1 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -176,24 +176,11 @@ static void ptn3460_post_disable(struct drm_bridge 
 *bridge)
  {
  }

 -static void ptn3460_bridge_destroy(struct drm_bridge *bridge)
 -{
 -   struct ptn3460_bridge

Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-02-02 Thread Ajay kumar 
Hi,

On Fri, Jan 30, 2015 at 9:29 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
 On Fri, Jan 30, 2015 at 10:37:19AM -0500, Rob Clark wrote:
 On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com 
 wrote:
 I'll also need to update the new bridge in the msm edp code..
 although that isn't such a big deal if I knew how this was *supposed*
 to work.. since what is there now at least doesn't look right..

 I wonder whether the new dw-hdmi bridge code get fixed up too...
I think its already fixed. Check this:
http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-nextid=b5217bf4692218d202d3d2cd772864fa1e10be4d

Regards,
Ajay Kumar
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-01-30 Thread Rob Clark 
On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com wrote:
 Currently, third party bridge drivers(ptn3460) are dependent
 on the corresponding encoder driver init, since bridge driver
 needs a drm_device pointer to finish drm initializations.
 The encoder driver passes the drm_device pointer to the
 bridge driver. Because of this dependency, third party drivers
 like ptn3460 doesn't adhere to the driver model.

 In this patch, we reframe the bridge registration framework
 so that bridge initialization is split into 2 steps, and
 bridge registration happens independent of drm flow:
 --Step 1: gather all the bridge settings independent of drm and
   add the bridge onto a global list of bridges.
 --Step 2: when the encoder driver is probed, call drm_bridge_attach
   for the corresponding bridge so that the bridge receives
   drm_device pointer and continues with connector and other
   drm initializations.

 The old set of bridge helpers are removed, and a set of new helpers
 are added to accomplish the 2 step initialization.

 The bridge devices register themselves onto global list of bridges
 when they get probed by calling drm_bridge_add.

 The parent encoder driver waits till the bridge is available
 in the lookup table(by calling of_drm_find_bridge) and then
 continues with its initialization.

 The encoder driver should also call drm_bridge_attach to pass
 on the drm_device to the bridge object.

 drm_bridge_attach inturn calls bridge-funcs-attach so that
 bridge can continue with drm related initializations.

ok, so I probably should have had a closer look at this before it
landed in drm-next, so if it is too late to revert (and deal w/
untangling subsequent patches that depend on this) some of these
issues we'll just have to fix with follow-on patches.

1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
in drm.tmpl
2) as far as I can tell, the new approach to cleaning up bridge
objects is to just let them leak !?!

I'll also need to update the new bridge in the msm edp code..
although that isn't such a big deal if I knew how this was *supposed*
to work.. since what is there now at least doesn't look right..

BR,
-R



 Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Tested-by: Rahul Sharma rahul.sha...@samsung.com
 Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Tested-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Tested-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
 ---
  drivers/gpu/drm/Makefile   |2 +-
  drivers/gpu/drm/bridge/ptn3460.c   |   27 +-
  drivers/gpu/drm/drm_bridge.c   |   91 
 
  drivers/gpu/drm/drm_crtc.c |   67 ---
  drivers/gpu/drm/msm/hdmi/hdmi.c|4 +-
  drivers/gpu/drm/msm/hdmi/hdmi.h|1 +
  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |6 +--
  drivers/gpu/drm/sti/sti_hda.c  |   10 +---
  drivers/gpu/drm/sti/sti_hdmi.c |   10 +---
  include/drm/bridge/ptn3460.h   |8 +++
  include/drm/drm_crtc.h |   26 -
  11 files changed, 133 insertions(+), 119 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_bridge.c

 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index e620807..c83ef2d 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -14,7 +14,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
 drm_info.o drm_debugfs.o drm_encoder_slave.o \
 drm_trace_points.o drm_global.o drm_prime.o \
 drm_rect.o drm_vma_manager.o drm_flip_work.o \
 -   drm_modeset_lock.o drm_atomic.o
 +   drm_modeset_lock.o drm_atomic.o drm_bridge.o

  drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index a2ddc8d..4a818c1 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -176,24 +176,11 @@ static void ptn3460_post_disable(struct drm_bridge 
 *bridge)
  {
  }

 -static void ptn3460_bridge_destroy(struct drm_bridge *bridge)
 -{
 -   struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
 -
 -   drm_bridge_cleanup(bridge);
 -   if (gpio_is_valid(ptn_bridge-gpio_pd_n))
 -   gpio_free(ptn_bridge-gpio_pd_n);
 -   if (gpio_is_valid(ptn_bridge-gpio_rst_n))
 -   gpio_free(ptn_bridge-gpio_rst_n);
 -   /* Nothing else to free, we've got devm allocated memory */
 -}
 -
  static struct drm_bridge_funcs ptn3460_bridge_funcs = {
 .pre_enable = ptn3460_pre_enable,
 .enable = ptn3460_enable,
 .disable = ptn3460_disable,
 .post_disable = ptn3460_post_disable,
 -   .destroy = ptn3460_bridge_destroy,
  };

  static int ptn3460_get_modes(struct

Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-01-30 Thread Russell King - ARM Linux 
On Fri, Jan 30, 2015 at 10:37:19AM -0500, Rob Clark wrote:
 On Tue, Jan 20, 2015 at 11:38 AM, Ajay Kumar ajaykumar...@samsung.com wrote:
 I'll also need to update the new bridge in the msm edp code..
 although that isn't such a big deal if I knew how this was *supposed*
 to work.. since what is there now at least doesn't look right..

I wonder whether the new dw-hdmi bridge code get fixed up too...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-01-30 Thread Daniel Stone 
Hi,

On 30 January 2015 at 15:37, Rob Clark robdcl...@gmail.com wrote:
 ok, so I probably should have had a closer look at this before it
 landed in drm-next, so if it is too late to revert (and deal w/
 untangling subsequent patches that depend on this) some of these
 issues we'll just have to fix with follow-on patches.

 1) needs headerdoc for new fxns in drm_bridge.c, and needs to be added
 in drm.tmpl
 2) as far as I can tell, the new approach to cleaning up bridge
 objects is to just let them leak !?!

 I'll also need to update the new bridge in the msm edp code..
 although that isn't such a big deal if I knew how this was *supposed*
 to work.. since what is there now at least doesn't look right..

Given how long these patches sat around doing being passively NACKed
by discussions going around in circles, and how useful they are, I'd
be much more in favour of fixing them up than reverting and going
through the whole circus again ...

Cheers,
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/14] drm/bridge: make bridge registration independent of drm flow

2015-01-20 Thread Ajay Kumar 
Currently, third party bridge drivers(ptn3460) are dependent
on the corresponding encoder driver init, since bridge driver
needs a drm_device pointer to finish drm initializations.
The encoder driver passes the drm_device pointer to the
bridge driver. Because of this dependency, third party drivers
like ptn3460 doesn't adhere to the driver model.

In this patch, we reframe the bridge registration framework
so that bridge initialization is split into 2 steps, and
bridge registration happens independent of drm flow:
--Step 1: gather all the bridge settings independent of drm and
  add the bridge onto a global list of bridges.
--Step 2: when the encoder driver is probed, call drm_bridge_attach
  for the corresponding bridge so that the bridge receives
  drm_device pointer and continues with connector and other
  drm initializations.

The old set of bridge helpers are removed, and a set of new helpers
are added to accomplish the 2 step initialization.

The bridge devices register themselves onto global list of bridges
when they get probed by calling drm_bridge_add.

The parent encoder driver waits till the bridge is available
in the lookup table(by calling of_drm_find_bridge) and then
continues with its initialization.

The encoder driver should also call drm_bridge_attach to pass
on the drm_device to the bridge object.

drm_bridge_attach inturn calls bridge-funcs-attach so that
bridge can continue with drm related initializations.

Signed-off-by: Ajay Kumar ajaykumar...@samsung.com
Acked-by: Inki Dae inki@samsung.com
Tested-by: Rahul Sharma rahul.sha...@samsung.com
Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Tested-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
Tested-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
---
 drivers/gpu/drm/Makefile   |2 +-
 drivers/gpu/drm/bridge/ptn3460.c   |   27 +-
 drivers/gpu/drm/drm_bridge.c   |   91 
 drivers/gpu/drm/drm_crtc.c |   67 ---
 drivers/gpu/drm/msm/hdmi/hdmi.c|4 +-
 drivers/gpu/drm/msm/hdmi/hdmi.h|1 +
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |6 +--
 drivers/gpu/drm/sti/sti_hda.c  |   10 +---
 drivers/gpu/drm/sti/sti_hdmi.c |   10 +---
 include/drm/bridge/ptn3460.h   |8 +++
 include/drm/drm_crtc.h |   26 -
 11 files changed, 133 insertions(+), 119 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_bridge.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e620807..c83ef2d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -14,7 +14,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
-   drm_modeset_lock.o drm_atomic.o
+   drm_modeset_lock.o drm_atomic.o drm_bridge.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index a2ddc8d..4a818c1 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -176,24 +176,11 @@ static void ptn3460_post_disable(struct drm_bridge 
*bridge)
 {
 }
 
-static void ptn3460_bridge_destroy(struct drm_bridge *bridge)
-{
-   struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge);
-
-   drm_bridge_cleanup(bridge);
-   if (gpio_is_valid(ptn_bridge-gpio_pd_n))
-   gpio_free(ptn_bridge-gpio_pd_n);
-   if (gpio_is_valid(ptn_bridge-gpio_rst_n))
-   gpio_free(ptn_bridge-gpio_rst_n);
-   /* Nothing else to free, we've got devm allocated memory */
-}
-
 static struct drm_bridge_funcs ptn3460_bridge_funcs = {
.pre_enable = ptn3460_pre_enable,
.enable = ptn3460_enable,
.disable = ptn3460_disable,
.post_disable = ptn3460_post_disable,
-   .destroy = ptn3460_bridge_destroy,
 };
 
 static int ptn3460_get_modes(struct drm_connector *connector)
@@ -314,7 +301,7 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder 
*encoder,
}
 
ptn_bridge-bridge.funcs = ptn3460_bridge_funcs;
-   ret = drm_bridge_init(dev, ptn_bridge-bridge);
+   ret = drm_bridge_attach(dev, ptn_bridge-bridge);
if (ret) {
DRM_ERROR(Failed to initialize bridge with drm\n);
goto err;
@@ -343,3 +330,15 @@ err:
return ret;
 }
 EXPORT_SYMBOL(ptn3460_init);
+
+void ptn3460_destroy(struct drm_bridge *bridge)
+{
+   struct ptn3460_bridge *ptn_bridge = bridge-driver_private;
+
+   if (gpio_is_valid(ptn_bridge-gpio_pd_n))
+   gpio_free(ptn_bridge-gpio_pd_n);
+   if (gpio_is_valid(ptn_bridge-gpio_rst_n))
+