Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-08 Thread Rahul Sharma
On Thu, Mar 7, 2013 at 2:05 PM, Inki Dae inki@samsung.com wrote:


 -Original Message-
 From: 김승우 [mailto:sw0312@samsung.com]
 Sent: Thursday, March 07, 2013 4:04 PM
 To: Rahul Sharma
 Cc: Inki Dae; linux-samsung-...@vger.kernel.org; Sean Paul; sunil joshi;
 dri-devel@lists.freedesktop.org; Rahul Sharma; sw0312@samsung.com
 Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to
 hdmiphy driver



 On 2013년 03월 07일 15:45, Rahul Sharma wrote:
  Thanks Seung Woo, Mr. Dae,
 
  On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote:
  2013/3/7 김승우 sw0312@samsung.com:
 
 
  On 2013년 03월 04일 23:05, Rahul Sharma wrote:
  Thanks Sean,
 
  On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com
 wrote:
  On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma
 rahul.sha...@samsung.com wrote:
  Right now hdmiphy operations and configs are kept inside hdmi
 driver. hdmiphy related
  code is tightly coupled with hdmi ip driver. Physicaly they are
 different devices and
 
  s/Physicaly/Physically/
 
  should be instantiated independently.
 
  In terms of versions/mapping/configurations Hdmi and hdmiphy are
 independent of each
  other. It is preferred to isolate them and maintained
 independently.
 
  This implementations is tested for:
  1) Resolutions supported by exynos4 and 5 hdmi.
  2) Runtime PM and S2R scenarions for exynos5.
 
 
  I don't like the idea of spawning off yet another driver in here. It
  adds more globals, more suspend/resume ordering issues, and more
  implicit dependencies. I understand, however, that this is the
 Chosen
  Way for the exynos driver, so I will save my rant.
 
 
  I agree to it. splitting phy to a new driver will complicate the
 power related
  scenarios. But in upcoming SoC,s, Phy is changing considerably in
 terms of
  config values, mapping (i2c/platform bus) etc. Handling this
 diversity
  inside hdmi driver is complicating it with unrelated changes.
 
  Basically, I agree with the idea to split hdmiphy from hdmi. And it
  seems that already existing hdmiphy i2c device is just reused and
  hdmiphy_power_on is reorganized to hdmiphy dpms operation: even
 calling
  flow of power operations is reordered.
 
  But I'm not sure exynos_hdmiphy_driver_register() really need to be
  called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough
 to
  call exynos_hdmiphy_driver_register() from hdmi_probe() because
 hdmiphy
  is only used from hdmi.
 
 
  I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.
 
 
  I agree to the Seung Woo's point that hdmi-phy used to be solely
 accessed by
  hdmi driver.  But in this RFC, hdmi-phy is not called by hdmi driver
  anymore. Phy
  ops will be called from drm-common-hdmi platform driver along with mixer
 and
  hdmi ops.

 Considering this, exynos_drm_hdmi_probe() is more proper position.

 
  The rational behind my implementation is that I am projecting hdmi-phy
 as
  a device which is peer to hdmi ip and mixer. These 3 devices together
 makes
  DRM HDMI subsystem.
 
  Even physically hdmi-phy doesn't seems to be a part of hdmi ip though
  configurations are listed under hdmi ip user manual. It looks like a
  isolated module accessed by i2c.
 
  Though I don't find anything wrong with Seung Woo suggestion but above
  placement of hdmi-phy (parallel to hdmi and mixer) makes more sense
  to me.
 
  Please have a another look at it and let me know your opinion.
 
  Another things which bothers me is registering mixer, hdmi driver inside
  exynos_drm_init(). If we strictly follow the hierarchy inside drm,
  exynos_drm_init()
  should register drm-common-hdmi only. drm-common-hdmi should register
  mixer and hdmi (or hdmi-phy as well).

 Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi,
 and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi
 for exynos).


 Ideally, right. But the reason I designed and implemented hdmi subsystem
 framework like this, is that there was one issue that
 platform_device_register() can't be called at probe(). On other words, when
 one platform device driver is being probed, anyone can't be probed. Anyway,
 existing way needs to be improved. So let's find better way and improve the
 hdmi subsystem framework this time. :)

Mr. Dae,

if we move exynos_drm_drv.o towards last in the makefile, we can
conveniently move all driver(or plf device for drm-comon-hdmi) registrations
from exynos_drm_init to module_init of the respective drivers.

I verified above changes for hdmi and S2R scenarios.

regards,
Rahul Sharma


 Thanks,
 Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim

 
  regards,
  Rahul Sharma.
 
  Thanks,
  Inki Dae
 
  Thanks and Regards,
  - Seung-Woo Kim
 
 
  I have tested this RFC for Runtime PM / S2R. But if we see any major
 roadblock
  we should re-factor this by explicitly calling power related
 callbacks
  of mixer, phy,
  hdmi drivers in a required order. We can call them from exynos-drm-
 hdmi

RE: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-07 Thread Inki Dae


 -Original Message-
 From: 김승우 [mailto:sw0312@samsung.com]
 Sent: Thursday, March 07, 2013 4:04 PM
 To: Rahul Sharma
 Cc: Inki Dae; linux-samsung-...@vger.kernel.org; Sean Paul; sunil joshi;
 dri-devel@lists.freedesktop.org; Rahul Sharma; sw0312@samsung.com
 Subject: Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to
 hdmiphy driver
 
 
 
 On 2013년 03월 07일 15:45, Rahul Sharma wrote:
  Thanks Seung Woo, Mr. Dae,
 
  On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote:
  2013/3/7 김승우 sw0312@samsung.com:
 
 
  On 2013년 03월 04일 23:05, Rahul Sharma wrote:
  Thanks Sean,
 
  On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com
 wrote:
  On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma
 rahul.sha...@samsung.com wrote:
  Right now hdmiphy operations and configs are kept inside hdmi
 driver. hdmiphy related
  code is tightly coupled with hdmi ip driver. Physicaly they are
 different devices and
 
  s/Physicaly/Physically/
 
  should be instantiated independently.
 
  In terms of versions/mapping/configurations Hdmi and hdmiphy are
 independent of each
  other. It is preferred to isolate them and maintained
independently.
 
  This implementations is tested for:
  1) Resolutions supported by exynos4 and 5 hdmi.
  2) Runtime PM and S2R scenarions for exynos5.
 
 
  I don't like the idea of spawning off yet another driver in here. It
  adds more globals, more suspend/resume ordering issues, and more
  implicit dependencies. I understand, however, that this is the
 Chosen
  Way for the exynos driver, so I will save my rant.
 
 
  I agree to it. splitting phy to a new driver will complicate the
 power related
  scenarios. But in upcoming SoC,s, Phy is changing considerably in
 terms of
  config values, mapping (i2c/platform bus) etc. Handling this
 diversity
  inside hdmi driver is complicating it with unrelated changes.
 
  Basically, I agree with the idea to split hdmiphy from hdmi. And it
  seems that already existing hdmiphy i2c device is just reused and
  hdmiphy_power_on is reorganized to hdmiphy dpms operation: even
 calling
  flow of power operations is reordered.
 
  But I'm not sure exynos_hdmiphy_driver_register() really need to be
  called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough
 to
  call exynos_hdmiphy_driver_register() from hdmi_probe() because
 hdmiphy
  is only used from hdmi.
 
 
  I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.
 
 
  I agree to the Seung Woo's point that hdmi-phy used to be solely
 accessed by
  hdmi driver.  But in this RFC, hdmi-phy is not called by hdmi driver
  anymore. Phy
  ops will be called from drm-common-hdmi platform driver along with mixer
 and
  hdmi ops.
 
 Considering this, exynos_drm_hdmi_probe() is more proper position.
 
 
  The rational behind my implementation is that I am projecting hdmi-phy
 as
  a device which is peer to hdmi ip and mixer. These 3 devices together
 makes
  DRM HDMI subsystem.
 
  Even physically hdmi-phy doesn't seems to be a part of hdmi ip though
  configurations are listed under hdmi ip user manual. It looks like a
  isolated module accessed by i2c.
 
  Though I don't find anything wrong with Seung Woo suggestion but above
  placement of hdmi-phy (parallel to hdmi and mixer) makes more sense
  to me.
 
  Please have a another look at it and let me know your opinion.
 
  Another things which bothers me is registering mixer, hdmi driver inside
  exynos_drm_init(). If we strictly follow the hierarchy inside drm,
  exynos_drm_init()
  should register drm-common-hdmi only. drm-common-hdmi should register
  mixer and hdmi (or hdmi-phy as well).
 
 Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi,
 and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi
 for exynos).
 

Ideally, right. But the reason I designed and implemented hdmi subsystem
framework like this, is that there was one issue that
platform_device_register() can't be called at probe(). On other words, when
one platform device driver is being probed, anyone can't be probed. Anyway,
existing way needs to be improved. So let's find better way and improve the
hdmi subsystem framework this time. :)

Thanks,
Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim
 
 
  regards,
  Rahul Sharma.
 
  Thanks,
  Inki Dae
 
  Thanks and Regards,
  - Seung-Woo Kim
 
 
  I have tested this RFC for Runtime PM / S2R. But if we see any major
 roadblock
  we should re-factor this by explicitly calling power related
 callbacks
  of mixer, phy,
  hdmi drivers in a required order. We can call them from exynos-drm-
 hdmi plf
  device. AFAIR something like this is already in place in chrome-
 kernel.
 
  I've made some comments below.
 
  This patch is dependent on
  http://www.mail-archive.com/dri-
 de...@lists.freedesktop.org/msg34733.html
  http://www.mail-archive.com/dri-
 de...@lists.freedesktop.org/msg34861.html
  http://www.mail-archive.com/dri-
 de

Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-07 Thread Rahul Sharma
On Thu, Mar 7, 2013 at 12:37 PM, Stéphane Marchesin
stephane.marche...@gmail.com wrote:
 On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae inki@samsung.com wrote:
 2013/3/7 김승우 sw0312@samsung.com:


 On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,

 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are 
 different devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.


 I agree to it. splitting phy to a new driver will complicate the power 
 related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

 Basically, I agree with the idea to split hdmiphy from hdmi. And it
 seems that already existing hdmiphy i2c device is just reused and
 hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
 flow of power operations is reordered.

 But I'm not sure exynos_hdmiphy_driver_register() really need to be
 called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
 call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
 is only used from hdmi.


 I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.


 But this is probably going to break dpms and/or suspend/resume
 functionality. Has that been tested?

 Stéphane

Hi Stéphane

This has been tested for dpms and suspend/resume scenarios for exynos5. I
have yet to try with hdmi, mixer, phy driver registration moved to
common-drm-hdmi.

regards,
Rahul Sharma.

 Thanks,
 Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim


 I have tested this RFC for Runtime PM / S2R. But if we see any major 
 roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.

 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


 snip

 --
 Seung-Woo Kim
 Samsung Software RD Center
 --

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
 --
 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



-- 
Regards,
Rahul Sharma,
email to: rahul.sha...@samsung.com
Samsung India.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-06 Thread 김승우


On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,
 
 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are different 
 devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.

 
 I agree to it. splitting phy to a new driver will complicate the power related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

Basically, I agree with the idea to split hdmiphy from hdmi. And it
seems that already existing hdmiphy i2c device is just reused and
hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
flow of power operations is reordered.

But I'm not sure exynos_hdmiphy_driver_register() really need to be
called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
is only used from hdmi.

Thanks and Regards,
- Seung-Woo Kim

 
 I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.
 
 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


snip

-- 
Seung-Woo Kim
Samsung Software RD Center
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-06 Thread Inki Dae
2013/3/7 김승우 sw0312@samsung.com:


 On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,

 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are different 
 devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.


 I agree to it. splitting phy to a new driver will complicate the power 
 related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

 Basically, I agree with the idea to split hdmiphy from hdmi. And it
 seems that already existing hdmiphy i2c device is just reused and
 hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
 flow of power operations is reordered.

 But I'm not sure exynos_hdmiphy_driver_register() really need to be
 called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
 call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
 is only used from hdmi.


I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.

Thanks,
Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim


 I have tested this RFC for Runtime PM / S2R. But if we see any major 
 roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.

 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


 snip

 --
 Seung-Woo Kim
 Samsung Software RD Center
 --

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-06 Thread Rahul Sharma
Thanks Seung Woo, Mr. Dae,

On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote:
 2013/3/7 김승우 sw0312@samsung.com:


 On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,

 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are different 
 devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.


 I agree to it. splitting phy to a new driver will complicate the power 
 related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

 Basically, I agree with the idea to split hdmiphy from hdmi. And it
 seems that already existing hdmiphy i2c device is just reused and
 hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
 flow of power operations is reordered.

 But I'm not sure exynos_hdmiphy_driver_register() really need to be
 called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
 call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
 is only used from hdmi.


 I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.


I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by
hdmi driver.  But in this RFC, hdmi-phy is not called by hdmi driver
anymore. Phy
ops will be called from drm-common-hdmi platform driver along with mixer and
hdmi ops.

The rational behind my implementation is that I am projecting hdmi-phy as
a device which is peer to hdmi ip and mixer. These 3 devices together makes
DRM HDMI subsystem.

Even physically hdmi-phy doesn't seems to be a part of hdmi ip though
configurations are listed under hdmi ip user manual. It looks like a
isolated module accessed by i2c.

Though I don't find anything wrong with Seung Woo suggestion but above
placement of hdmi-phy (parallel to hdmi and mixer) makes more sense
to me.

Please have a another look at it and let me know your opinion.

Another things which bothers me is registering mixer, hdmi driver inside
exynos_drm_init(). If we strictly follow the hierarchy inside drm,
exynos_drm_init()
should register drm-common-hdmi only. drm-common-hdmi should register
mixer and hdmi (or hdmi-phy as well).

regards,
Rahul Sharma.

 Thanks,
 Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim


 I have tested this RFC for Runtime PM / S2R. But if we see any major 
 roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.

 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


 snip

 --
 Seung-Woo Kim
 Samsung Software RD Center
 --

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Regards,
Rahul Sharma,
email to: rahul.sha...@samsung.com
Samsung India.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-06 Thread 김승우


On 2013년 03월 07일 15:45, Rahul Sharma wrote:
 Thanks Seung Woo, Mr. Dae,
 
 On Thu, Mar 7, 2013 at 10:13 AM, Inki Dae inki@samsung.com wrote:
 2013/3/7 김승우 sw0312@samsung.com:


 On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,

 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are 
 different devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.


 I agree to it. splitting phy to a new driver will complicate the power 
 related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

 Basically, I agree with the idea to split hdmiphy from hdmi. And it
 seems that already existing hdmiphy i2c device is just reused and
 hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
 flow of power operations is reordered.

 But I'm not sure exynos_hdmiphy_driver_register() really need to be
 called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
 call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
 is only used from hdmi.


 I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.

 
 I agree to the Seung Woo's point that hdmi-phy used to be solely accessed by
 hdmi driver.  But in this RFC, hdmi-phy is not called by hdmi driver
 anymore. Phy
 ops will be called from drm-common-hdmi platform driver along with mixer and
 hdmi ops.

Considering this, exynos_drm_hdmi_probe() is more proper position.

 
 The rational behind my implementation is that I am projecting hdmi-phy as
 a device which is peer to hdmi ip and mixer. These 3 devices together makes
 DRM HDMI subsystem.
 
 Even physically hdmi-phy doesn't seems to be a part of hdmi ip though
 configurations are listed under hdmi ip user manual. It looks like a
 isolated module accessed by i2c.
 
 Though I don't find anything wrong with Seung Woo suggestion but above
 placement of hdmi-phy (parallel to hdmi and mixer) makes more sense
 to me.
 
 Please have a another look at it and let me know your opinion.
 
 Another things which bothers me is registering mixer, hdmi driver inside
 exynos_drm_init(). If we strictly follow the hierarchy inside drm,
 exynos_drm_init()
 should register drm-common-hdmi only. drm-common-hdmi should register
 mixer and hdmi (or hdmi-phy as well).

Yes, it makes sense. All real hw blocks for hdmi including mixer, hdmi,
and hdmiphy shoulde be registered in exynos_drm_hdmi (drm-common-hdmi
for exynos).

Thanks and Regards,
- Seung-Woo Kim

 
 regards,
 Rahul Sharma.
 
 Thanks,
 Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim


 I have tested this RFC for Runtime PM / S2R. But if we see any major 
 roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.

 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


 snip

 --
 Seung-Woo Kim
 Samsung Software RD Center
 --

 ___
 dri-devel mailing 

Re: [PATCH RFC] drm/exynos: hdmi: move hdmiphy related code to hdmiphy driver

2013-03-06 Thread Stéphane Marchesin
On Wed, Mar 6, 2013 at 8:43 PM, Inki Dae inki@samsung.com wrote:
 2013/3/7 김승우 sw0312@samsung.com:


 On 2013년 03월 04일 23:05, Rahul Sharma wrote:
 Thanks Sean,

 On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul seanp...@google.com wrote:
 On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma rahul.sha...@samsung.com 
 wrote:
 Right now hdmiphy operations and configs are kept inside hdmi driver. 
 hdmiphy related
 code is tightly coupled with hdmi ip driver. Physicaly they are different 
 devices and

 s/Physicaly/Physically/

 should be instantiated independently.

 In terms of versions/mapping/configurations Hdmi and hdmiphy are 
 independent of each
 other. It is preferred to isolate them and maintained independently.

 This implementations is tested for:
 1) Resolutions supported by exynos4 and 5 hdmi.
 2) Runtime PM and S2R scenarions for exynos5.


 I don't like the idea of spawning off yet another driver in here. It
 adds more globals, more suspend/resume ordering issues, and more
 implicit dependencies. I understand, however, that this is the Chosen
 Way for the exynos driver, so I will save my rant.


 I agree to it. splitting phy to a new driver will complicate the power 
 related
 scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of
 config values, mapping (i2c/platform bus) etc. Handling this diversity
 inside hdmi driver is complicating it with unrelated changes.

 Basically, I agree with the idea to split hdmiphy from hdmi. And it
 seems that already existing hdmiphy i2c device is just reused and
 hdmiphy_power_on is reorganized to hdmiphy dpms operation: even calling
 flow of power operations is reordered.

 But I'm not sure exynos_hdmiphy_driver_register() really need to be
 called from exynos_drm_init() of exynos_drm_drv.c. IMO, it is enough to
 call exynos_hdmiphy_driver_register() from hdmi_probe() because hdmiphy
 is only used from hdmi.


 I agree with Seung-Woo. The hdmiphy is just one part of HDMI subsystem.


But this is probably going to break dpms and/or suspend/resume
functionality. Has that been tested?

Stéphane

 Thanks,
 Inki Dae

 Thanks and Regards,
 - Seung-Woo Kim


 I have tested this RFC for Runtime PM / S2R. But if we see any major 
 roadblock
 we should re-factor this by explicitly calling power related callbacks
 of mixer, phy,
 hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf
 device. AFAIR something like this is already in place in chrome-kernel.

 I've made some comments below.

 This patch is dependent on
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34733.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34861.html
 http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg34862.html

 Signed-off-by: Rahul Sharma rahul.sha...@samsung.com
 ---
 It is based on exynos-drm-next-todo branch at
 git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git

  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   8 +
  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   6 +
  drivers/gpu/drm/exynos/exynos_drm_hdmi.c |  58 ++-
  drivers/gpu/drm/exynos/exynos_drm_hdmi.h |  11 +
  drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++--
  drivers/gpu/drm/exynos/exynos_hdmi.h |   1 -
  drivers/gpu/drm/exynos/exynos_hdmiphy.c  | 586 
 ++-
  drivers/gpu/drm/exynos/regs-hdmiphy.h|  61 
  8 files changed, 738 insertions(+), 368 deletions(-)
  create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h


 snip

 --
 Seung-Woo Kim
 Samsung Software RD Center
 --

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel