Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-31 Thread Joonyoung Shim
On 05/30/2015 06:33 AM, Gustavo Padovan wrote:
 2015-05-29 Joonyoung Shim jy0922.s...@samsung.com:
 
 On 05/29/2015 06:36 AM, Gustavo Padovan wrote:
 2015-05-28 Joonyoung Shim jy0922.s...@samsung.com:

 On 05/28/2015 05:24 PM, Joonyoung Shim wrote:
 On 05/28/2015 02:39 PM, Inki Dae wrote:
 Hi Gustavo,

 On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
 Hi Inki,

 2015-05-27 Inki Dae inki@samsung.com:

 Hi Gustavo,

 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk

 Run dpms operations through the atomic intefaces. This basically 
 removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.

 v2: Address comments by Joonyoung:
   - make hdmi code call -disable() instead of -dpms()
   - do not use WARN_ON on crtc enable/disable

 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge

 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/ps8622.c 
 b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ps8622_detect,
   .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ptn3460_detect,
   .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

 [--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 - .dpms   = exynos_drm_crtc_dpms,
 - .prepare= exynos_drm_crtc_prepare,
 - .commit = exynos_drm_crtc_commit,
 + .enable = exynos_drm_crtc_enable,
 + .disable= exynos_drm_crtc_disable,
   .mode_fixup = exynos_drm_crtc_mode_fixup,
   .mode_set   = drm_helper_crtc_mode_set,
   .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

 I think it'd be better to use atomic_flush callback to enable global 
 dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?

 atomic relevant codes I looked into do as follows,

 atomic_begin();

 atomic_update();  /* this will call win_commit callback to set a 
 overlay
 relevant registers and enable its dma channel. */

 atomic_flush();

 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.

 I think we can go down that road, but I'd suggest we push the atomic
 patches v8 (with the lastest comments from Joonyoung fixed) and then 
 work on the change you are proposing as a follow-up together with the 
 other improvements for atomic I already have queued here. This way
 we don't take the risk of missing one more merge window.

 We(I and Joonyoung) will have discussion about this patch series. For
 this, we have already started to analyze entire atomic features
 including your patch set so I'd merge it at end of next week according
 to the discussion. I'm not kind of sure yet but I will merge it as long
 as there is no big problem.


 Actually i agree to opinion of Gustavo and will repost the patchset of
 Gustavo with some 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-29 Thread Joonyoung Shim
On 05/29/2015 06:36 AM, Gustavo Padovan wrote:
 2015-05-28 Joonyoung Shim jy0922.s...@samsung.com:
 
 On 05/28/2015 05:24 PM, Joonyoung Shim wrote:
 On 05/28/2015 02:39 PM, Inki Dae wrote:
 Hi Gustavo,

 On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
 Hi Inki,

 2015-05-27 Inki Dae inki@samsung.com:

 Hi Gustavo,

 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk

 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.

 v2: Address comments by Joonyoung:
 - make hdmi code call -disable() instead of -dpms()
 - do not use WARN_ON on crtc enable/disable

 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge

 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/ps8622.c 
 b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 -   .dpms = drm_helper_connector_dpms,
 +   .dpms = drm_atomic_helper_connector_dpms,
 .fill_modes = drm_helper_probe_single_connector_modes,
 .detect = ps8622_detect,
 .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 -   .dpms = drm_helper_connector_dpms,
 +   .dpms = drm_atomic_helper_connector_dpms,
 .fill_modes = drm_helper_probe_single_connector_modes,
 .detect = ptn3460_detect,
 .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

 [--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 -   .dpms   = exynos_drm_crtc_dpms,
 -   .prepare= exynos_drm_crtc_prepare,
 -   .commit = exynos_drm_crtc_commit,
 +   .enable = exynos_drm_crtc_enable,
 +   .disable= exynos_drm_crtc_disable,
 .mode_fixup = exynos_drm_crtc_mode_fixup,
 .mode_set   = drm_helper_crtc_mode_set,
 .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

 I think it'd be better to use atomic_flush callback to enable global dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?

 atomic relevant codes I looked into do as follows,

 atomic_begin();

 atomic_update();  /* this will call win_commit callback to set a overlay
 relevant registers and enable its dma channel. */

 atomic_flush();

 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.

 I think we can go down that road, but I'd suggest we push the atomic
 patches v8 (with the lastest comments from Joonyoung fixed) and then 
 work on the change you are proposing as a follow-up together with the 
 other improvements for atomic I already have queued here. This way
 we don't take the risk of missing one more merge window.

 We(I and Joonyoung) will have discussion about this patch series. For
 this, we have already started to analyze entire atomic features
 including your patch set so I'd merge it at end of next week according
 to the discussion. I'm not kind of sure yet but I will merge it as long
 as there is no big problem.


 Actually i agree to opinion of Gustavo and will repost the patchset of
 Gustavo with some patches fixed by me.


 Hmm, i meet problem of operations order. It's 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-29 Thread Gustavo Padovan
2015-05-29 Joonyoung Shim jy0922.s...@samsung.com:

 On 05/29/2015 06:36 AM, Gustavo Padovan wrote:
  2015-05-28 Joonyoung Shim jy0922.s...@samsung.com:
  
  On 05/28/2015 05:24 PM, Joonyoung Shim wrote:
  On 05/28/2015 02:39 PM, Inki Dae wrote:
  Hi Gustavo,
 
  On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
  Hi Inki,
 
  2015-05-27 Inki Dae inki@samsung.com:
 
  Hi Gustavo,
 
  On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
  Run dpms operations through the atomic intefaces. This basically 
  removes
  the .dpms() callback from econders and crtcs and use .disable() and
  .enable() to turn the crtc on and off.
 
  v2: Address comments by Joonyoung:
- make hdmi code call -disable() instead of -dpms()
- do not use WARN_ON on crtc enable/disable
 
  v3: - Fix build failure after the hdmi change in v2
  - Change dpms helper of ptn3460 bridge
 
  Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
  Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
  Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
  ---
   drivers/gpu/drm/bridge/ps8622.c |  2 +-
   drivers/gpu/drm/bridge/ptn3460.c|  2 +-
   drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
  -
   drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
   drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
   drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
   drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
   10 files changed, 69 insertions(+), 75 deletions(-)
 
  diff --git a/drivers/gpu/drm/bridge/ps8622.c 
  b/drivers/gpu/drm/bridge/ps8622.c
  index b604326..d686235 100644
  --- a/drivers/gpu/drm/bridge/ps8622.c
  +++ b/drivers/gpu/drm/bridge/ps8622.c
  @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static const struct drm_connector_funcs ps8622_connector_funcs = {
  - .dpms = drm_helper_connector_dpms,
  + .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ps8622_detect,
.destroy = ps8622_connector_destroy,
  diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
  b/drivers/gpu/drm/bridge/ptn3460.c
  index 8ed3617..260bc9f 100644
  --- a/drivers/gpu/drm/bridge/ptn3460.c
  +++ b/drivers/gpu/drm/bridge/ptn3460.c
  @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static struct drm_connector_funcs ptn3460_connector_funcs = {
  - .dpms = drm_helper_connector_dpms,
  + .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ptn3460_detect,
.destroy = ptn3460_connector_destroy,
  diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
  b/drivers/gpu/drm/exynos/exynos_dp_core.c
  index 195fe60..c9995b1 100644
  --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
  +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
  @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
  drm_connector *connector)
   }
   
 
  [--snip--]
 
   
   static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
  - .dpms   = exynos_drm_crtc_dpms,
  - .prepare= exynos_drm_crtc_prepare,
  - .commit = exynos_drm_crtc_commit,
  + .enable = exynos_drm_crtc_enable,
  + .disable= exynos_drm_crtc_disable,
.mode_fixup = exynos_drm_crtc_mode_fixup,
.mode_set   = drm_helper_crtc_mode_set,
.mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,
 
  I think it'd be better to use atomic_flush callback to enable global 
  dma
  like commit callback did. Is there any reason that you don't use
  atomic_begin and atomic_flush callbacks?
 
  atomic relevant codes I looked into do as follows,
 
  atomic_begin();
 
  atomic_update();  /* this will call win_commit callback to set a 
  overlay
  relevant registers and enable its dma channel. */
 
  atomic_flush();
 
  So atomic overlay updating between atomic_begin() ~ atomic_flush() will
  be guaranteed.
 
  I think we can go down that road, but I'd suggest we push the atomic
  patches v8 (with the lastest comments from Joonyoung fixed) and then 
  work on the change you are proposing as a follow-up together with the 
  other improvements for atomic I already have queued here. This way
  we don't take the risk of missing one more merge window.
 
  We(I and Joonyoung) will have discussion about this patch series. For
  this, we have already started to analyze entire atomic features
  including your patch set so I'd merge it at end of next week according
  to the discussion. I'm not kind of sure yet but I will merge it as long
  as there is no big problem.
 
 
  Actually i 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-28 Thread Gustavo Padovan
2015-05-28 Joonyoung Shim jy0922.s...@samsung.com:

 On 05/28/2015 05:24 PM, Joonyoung Shim wrote:
  On 05/28/2015 02:39 PM, Inki Dae wrote:
  Hi Gustavo,
 
  On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
  Hi Inki,
 
  2015-05-27 Inki Dae inki@samsung.com:
 
  Hi Gustavo,
 
  On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
  Run dpms operations through the atomic intefaces. This basically removes
  the .dpms() callback from econders and crtcs and use .disable() and
  .enable() to turn the crtc on and off.
 
  v2: Address comments by Joonyoung:
  - make hdmi code call -disable() instead of -dpms()
  - do not use WARN_ON on crtc enable/disable
 
  v3: - Fix build failure after the hdmi change in v2
  - Change dpms helper of ptn3460 bridge
 
  Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
  Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
  Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
  ---
   drivers/gpu/drm/bridge/ps8622.c |  2 +-
   drivers/gpu/drm/bridge/ptn3460.c|  2 +-
   drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
  -
   drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
   drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
   drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
   drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
   10 files changed, 69 insertions(+), 75 deletions(-)
 
  diff --git a/drivers/gpu/drm/bridge/ps8622.c 
  b/drivers/gpu/drm/bridge/ps8622.c
  index b604326..d686235 100644
  --- a/drivers/gpu/drm/bridge/ps8622.c
  +++ b/drivers/gpu/drm/bridge/ps8622.c
  @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static const struct drm_connector_funcs ps8622_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ps8622_detect,
  .destroy = ps8622_connector_destroy,
  diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
  b/drivers/gpu/drm/bridge/ptn3460.c
  index 8ed3617..260bc9f 100644
  --- a/drivers/gpu/drm/bridge/ptn3460.c
  +++ b/drivers/gpu/drm/bridge/ptn3460.c
  @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static struct drm_connector_funcs ptn3460_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ptn3460_detect,
  .destroy = ptn3460_connector_destroy,
  diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
  b/drivers/gpu/drm/exynos/exynos_dp_core.c
  index 195fe60..c9995b1 100644
  --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
  +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
  @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
  drm_connector *connector)
   }
   
 
  [--snip--]
 
   
   static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
  -   .dpms   = exynos_drm_crtc_dpms,
  -   .prepare= exynos_drm_crtc_prepare,
  -   .commit = exynos_drm_crtc_commit,
  +   .enable = exynos_drm_crtc_enable,
  +   .disable= exynos_drm_crtc_disable,
  .mode_fixup = exynos_drm_crtc_mode_fixup,
  .mode_set   = drm_helper_crtc_mode_set,
  .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,
 
  I think it'd be better to use atomic_flush callback to enable global dma
  like commit callback did. Is there any reason that you don't use
  atomic_begin and atomic_flush callbacks?
 
  atomic relevant codes I looked into do as follows,
 
  atomic_begin();
 
  atomic_update();  /* this will call win_commit callback to set a overlay
  relevant registers and enable its dma channel. */
 
  atomic_flush();
 
  So atomic overlay updating between atomic_begin() ~ atomic_flush() will
  be guaranteed.
 
  I think we can go down that road, but I'd suggest we push the atomic
  patches v8 (with the lastest comments from Joonyoung fixed) and then 
  work on the change you are proposing as a follow-up together with the 
  other improvements for atomic I already have queued here. This way
  we don't take the risk of missing one more merge window.
 
  We(I and Joonyoung) will have discussion about this patch series. For
  this, we have already started to analyze entire atomic features
  including your patch set so I'd merge it at end of next week according
  to the discussion. I'm not kind of sure yet but I will merge it as long
  as there is no big problem.
 
  
  Actually i agree to opinion of Gustavo and will repost the patchset of
  Gustavo 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-28 Thread Joonyoung Shim
On 05/28/2015 02:39 PM, Inki Dae wrote:
 Hi Gustavo,
 
 On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
 Hi Inki,

 2015-05-27 Inki Dae inki@samsung.com:

 Hi Gustavo,

 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk

 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.

 v2: Address comments by Joonyoung:
- make hdmi code call -disable() instead of -dpms()
- do not use WARN_ON on crtc enable/disable

 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge

 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/ps8622.c 
 b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 -  .dpms = drm_helper_connector_dpms,
 +  .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ps8622_detect,
.destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 -  .dpms = drm_helper_connector_dpms,
 +  .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ptn3460_detect,
.destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

 [--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 -  .dpms   = exynos_drm_crtc_dpms,
 -  .prepare= exynos_drm_crtc_prepare,
 -  .commit = exynos_drm_crtc_commit,
 +  .enable = exynos_drm_crtc_enable,
 +  .disable= exynos_drm_crtc_disable,
.mode_fixup = exynos_drm_crtc_mode_fixup,
.mode_set   = drm_helper_crtc_mode_set,
.mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

 I think it'd be better to use atomic_flush callback to enable global dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?

 atomic relevant codes I looked into do as follows,

 atomic_begin();

 atomic_update();  /* this will call win_commit callback to set a overlay
 relevant registers and enable its dma channel. */

 atomic_flush();

 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.

 I think we can go down that road, but I'd suggest we push the atomic
 patches v8 (with the lastest comments from Joonyoung fixed) and then 
 work on the change you are proposing as a follow-up together with the 
 other improvements for atomic I already have queued here. This way
 we don't take the risk of missing one more merge window.
 
 We(I and Joonyoung) will have discussion about this patch series. For
 this, we have already started to analyze entire atomic features
 including your patch set so I'd merge it at end of next week according
 to the discussion. I'm not kind of sure yet but I will merge it as long
 as there is no big problem.
 

Actually i agree to opinion of Gustavo and will repost the patchset of
Gustavo with some patches fixed by me.

Thanks.
--
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 v7 12/13] drm/exynos: atomic dpms support

2015-05-28 Thread Joonyoung Shim
On 05/28/2015 05:24 PM, Joonyoung Shim wrote:
 On 05/28/2015 02:39 PM, Inki Dae wrote:
 Hi Gustavo,

 On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
 Hi Inki,

 2015-05-27 Inki Dae inki@samsung.com:

 Hi Gustavo,

 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk

 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.

 v2: Address comments by Joonyoung:
   - make hdmi code call -disable() instead of -dpms()
   - do not use WARN_ON on crtc enable/disable

 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge

 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/ps8622.c 
 b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ps8622_detect,
   .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ptn3460_detect,
   .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

 [--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 - .dpms   = exynos_drm_crtc_dpms,
 - .prepare= exynos_drm_crtc_prepare,
 - .commit = exynos_drm_crtc_commit,
 + .enable = exynos_drm_crtc_enable,
 + .disable= exynos_drm_crtc_disable,
   .mode_fixup = exynos_drm_crtc_mode_fixup,
   .mode_set   = drm_helper_crtc_mode_set,
   .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

 I think it'd be better to use atomic_flush callback to enable global dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?

 atomic relevant codes I looked into do as follows,

 atomic_begin();

 atomic_update();  /* this will call win_commit callback to set a overlay
 relevant registers and enable its dma channel. */

 atomic_flush();

 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.

 I think we can go down that road, but I'd suggest we push the atomic
 patches v8 (with the lastest comments from Joonyoung fixed) and then 
 work on the change you are proposing as a follow-up together with the 
 other improvements for atomic I already have queued here. This way
 we don't take the risk of missing one more merge window.

 We(I and Joonyoung) will have discussion about this patch series. For
 this, we have already started to analyze entire atomic features
 including your patch set so I'd merge it at end of next week according
 to the discussion. I'm not kind of sure yet but I will merge it as long
 as there is no big problem.

 
 Actually i agree to opinion of Gustavo and will repost the patchset of
 Gustavo with some patches fixed by me.
 

Hmm, i meet problem of operations order. It's called .atomic_update
before enable crtc and called .atomic_disable after disable crtc. It
means .win_commit and .win_disable just return 0 without any operations
because e.g. mixer_ctx-powered is 0 in mixer driver, so we 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-27 Thread Joonyoung Shim
Hi Gustavo,

Sorry, i was missing some review points.

On 05/23/2015 12:40 AM, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.
 
 v2: Address comments by Joonyoung:
   - make hdmi code call -disable() instead of -dpms()
   - do not use WARN_ON on crtc enable/disable
 
 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge
 
 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)
 
 diff --git a/drivers/gpu/drm/bridge/ps8622.c b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct drm_connector 
 *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ps8622_detect,
   .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ptn3460_detect,
   .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs exynos_dp_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = exynos_dp_detect,
   .destroy = exynos_dp_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index fd5ef2c..ca501a9 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -22,48 +22,54 @@
  #include exynos_drm_encoder.h
  #include exynos_drm_plane.h
  
 -static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
 +static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
  {
   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 + struct exynos_drm_plane *exynos_plane = to_exynos_plane(crtc-primary);
  
 - DRM_DEBUG_KMS(crtc[%d] mode[%d]\n, crtc-base.id, mode);
 -
 - if (exynos_crtc-dpms == mode) {
 - DRM_DEBUG_KMS(desired dpms mode is same as previous one.\n);
 + if (exynos_crtc-enabled)
   return;
 - }
 -
 - if (mode  DRM_MODE_DPMS_ON) {
 - /* wait for the completion of page flip. */
 - if (!wait_event_timeout(exynos_crtc-pending_flip_queue,
 - (exynos_crtc-event == NULL), HZ/20))
 - exynos_crtc-event = NULL;
 - drm_crtc_vblank_off(crtc);
 - }
  
   if (exynos_crtc-ops-dpms)
 - exynos_crtc-ops-dpms(exynos_crtc, mode);
 + exynos_crtc-ops-dpms(exynos_crtc, DRM_MODE_DPMS_ON);
  
 - exynos_crtc-dpms = mode;
 + exynos_crtc-enabled = true;
  
 - if (mode == DRM_MODE_DPMS_ON)
 - drm_crtc_vblank_on(crtc);
 -}
 + drm_crtc_vblank_on(crtc);
  
 -static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
 -{
 - /* drm framework doesn't check NULL. */
 + if (exynos_crtc-ops-win_commit)
 + exynos_crtc-ops-win_commit(exynos_crtc, exynos_plane-zpos);

Unnecessary also? because be called by 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-27 Thread Inki Dae
Hi Gustavo,

On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.
 
 v2: Address comments by Joonyoung:
   - make hdmi code call -disable() instead of -dpms()
   - do not use WARN_ON on crtc enable/disable
 
 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge
 
 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)
 
 diff --git a/drivers/gpu/drm/bridge/ps8622.c b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct drm_connector 
 *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ps8622_detect,
   .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 - .dpms = drm_helper_connector_dpms,
 + .dpms = drm_atomic_helper_connector_dpms,
   .fill_modes = drm_helper_probe_single_connector_modes,
   .detect = ptn3460_detect,
   .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

[--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 - .dpms   = exynos_drm_crtc_dpms,
 - .prepare= exynos_drm_crtc_prepare,
 - .commit = exynos_drm_crtc_commit,
 + .enable = exynos_drm_crtc_enable,
 + .disable= exynos_drm_crtc_disable,
   .mode_fixup = exynos_drm_crtc_mode_fixup,
   .mode_set   = drm_helper_crtc_mode_set,
   .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

I think it'd be better to use atomic_flush callback to enable global dma
like commit callback did. Is there any reason that you don't use
atomic_begin and atomic_flush callbacks?

atomic relevant codes I looked into do as follows,

atomic_begin();

atomic_update();  /* this will call win_commit callback to set a overlay
relevant registers and enable its dma channel. */

atomic_flush();

So atomic overlay updating between atomic_begin() ~ atomic_flush() will
be guaranteed.

Thanks,
Inki Dae

   .mode_set_base  = drm_helper_crtc_mode_set_base,
 - .disable= exynos_drm_crtc_disable,
 + .atomic_check   = exynos_crtc_atomic_check,
  };
  

[--snip--]

--
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 v7 12/13] drm/exynos: atomic dpms support

2015-05-27 Thread Gustavo Padovan
Hi Joonyoung,

2015-05-27 Joonyoung Shim jy0922.s...@samsung.com:

 Hi Gustavo,
 
 Sorry, i was missing some review points.
 
 On 05/23/2015 12:40 AM, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
  
  Run dpms operations through the atomic intefaces. This basically removes
  the .dpms() callback from econders and crtcs and use .disable() and
  .enable() to turn the crtc on and off.
  
  v2: Address comments by Joonyoung:
  - make hdmi code call -disable() instead of -dpms()
  - do not use WARN_ON on crtc enable/disable
  
  v3: - Fix build failure after the hdmi change in v2
  - Change dpms helper of ptn3460 bridge
  
  Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
  Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
  Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
  ---
   drivers/gpu/drm/bridge/ps8622.c |  2 +-
   drivers/gpu/drm/bridge/ptn3460.c|  2 +-
   drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
  -
   drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
   drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
   drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
   drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
   10 files changed, 69 insertions(+), 75 deletions(-)
  
  diff --git a/drivers/gpu/drm/bridge/ps8622.c 
  b/drivers/gpu/drm/bridge/ps8622.c
  index b604326..d686235 100644
  --- a/drivers/gpu/drm/bridge/ps8622.c
  +++ b/drivers/gpu/drm/bridge/ps8622.c
  @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static const struct drm_connector_funcs ps8622_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ps8622_detect,
  .destroy = ps8622_connector_destroy,
  diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
  b/drivers/gpu/drm/bridge/ptn3460.c
  index 8ed3617..260bc9f 100644
  --- a/drivers/gpu/drm/bridge/ptn3460.c
  +++ b/drivers/gpu/drm/bridge/ptn3460.c
  @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static struct drm_connector_funcs ptn3460_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ptn3460_detect,
  .destroy = ptn3460_connector_destroy,
  diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
  b/drivers/gpu/drm/exynos/exynos_dp_core.c
  index 195fe60..c9995b1 100644
  --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
  +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
  @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static struct drm_connector_funcs exynos_dp_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = exynos_dp_detect,
  .destroy = exynos_dp_connector_destroy,
  diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
  b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
  index fd5ef2c..ca501a9 100644
  --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
  +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
  @@ -22,48 +22,54 @@
   #include exynos_drm_encoder.h
   #include exynos_drm_plane.h
   
  -static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
  +static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
   {
  struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
  +   struct exynos_drm_plane *exynos_plane = to_exynos_plane(crtc-primary);
   
  -   DRM_DEBUG_KMS(crtc[%d] mode[%d]\n, crtc-base.id, mode);
  -
  -   if (exynos_crtc-dpms == mode) {
  -   DRM_DEBUG_KMS(desired dpms mode is same as previous one.\n);
  +   if (exynos_crtc-enabled)
  return;
  -   }
  -
  -   if (mode  DRM_MODE_DPMS_ON) {
  -   /* wait for the completion of page flip. */
  -   if (!wait_event_timeout(exynos_crtc-pending_flip_queue,
  -   (exynos_crtc-event == NULL), HZ/20))
  -   exynos_crtc-event = NULL;
  -   drm_crtc_vblank_off(crtc);
  -   }
   
  if (exynos_crtc-ops-dpms)
  -   exynos_crtc-ops-dpms(exynos_crtc, mode);
  +   exynos_crtc-ops-dpms(exynos_crtc, DRM_MODE_DPMS_ON);
   
  -   exynos_crtc-dpms = mode;
  +   exynos_crtc-enabled = true;
   
  -   if (mode == DRM_MODE_DPMS_ON)
  -   drm_crtc_vblank_on(crtc);
  -}
  +   drm_crtc_vblank_on(crtc);
   
  -static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
  -{
  -   /* drm framework doesn't check NULL. */
  +   if (exynos_crtc-ops-win_commit)
  + 

Re: [PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-27 Thread Gustavo Padovan
Hi Inki,

2015-05-27 Inki Dae inki@samsung.com:

 Hi Gustavo,
 
 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
  From: Gustavo Padovan gustavo.pado...@collabora.co.uk
  
  Run dpms operations through the atomic intefaces. This basically removes
  the .dpms() callback from econders and crtcs and use .disable() and
  .enable() to turn the crtc on and off.
  
  v2: Address comments by Joonyoung:
  - make hdmi code call -disable() instead of -dpms()
  - do not use WARN_ON on crtc enable/disable
  
  v3: - Fix build failure after the hdmi change in v2
  - Change dpms helper of ptn3460 bridge
  
  Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
  Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
  Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
  ---
   drivers/gpu/drm/bridge/ps8622.c |  2 +-
   drivers/gpu/drm/bridge/ptn3460.c|  2 +-
   drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
  -
   drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
   drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
   drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
   drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
   drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
   10 files changed, 69 insertions(+), 75 deletions(-)
  
  diff --git a/drivers/gpu/drm/bridge/ps8622.c 
  b/drivers/gpu/drm/bridge/ps8622.c
  index b604326..d686235 100644
  --- a/drivers/gpu/drm/bridge/ps8622.c
  +++ b/drivers/gpu/drm/bridge/ps8622.c
  @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static const struct drm_connector_funcs ps8622_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ps8622_detect,
  .destroy = ps8622_connector_destroy,
  diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
  b/drivers/gpu/drm/bridge/ptn3460.c
  index 8ed3617..260bc9f 100644
  --- a/drivers/gpu/drm/bridge/ptn3460.c
  +++ b/drivers/gpu/drm/bridge/ptn3460.c
  @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
  drm_connector *connector)
   }
   
   static struct drm_connector_funcs ptn3460_connector_funcs = {
  -   .dpms = drm_helper_connector_dpms,
  +   .dpms = drm_atomic_helper_connector_dpms,
  .fill_modes = drm_helper_probe_single_connector_modes,
  .detect = ptn3460_detect,
  .destroy = ptn3460_connector_destroy,
  diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
  b/drivers/gpu/drm/exynos/exynos_dp_core.c
  index 195fe60..c9995b1 100644
  --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
  +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
  @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
  drm_connector *connector)
   }
   
 
 [--snip--]
 
   
   static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
  -   .dpms   = exynos_drm_crtc_dpms,
  -   .prepare= exynos_drm_crtc_prepare,
  -   .commit = exynos_drm_crtc_commit,
  +   .enable = exynos_drm_crtc_enable,
  +   .disable= exynos_drm_crtc_disable,
  .mode_fixup = exynos_drm_crtc_mode_fixup,
  .mode_set   = drm_helper_crtc_mode_set,
  .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,
 
 I think it'd be better to use atomic_flush callback to enable global dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?
 
 atomic relevant codes I looked into do as follows,
 
 atomic_begin();
 
 atomic_update();  /* this will call win_commit callback to set a overlay
 relevant registers and enable its dma channel. */
 
 atomic_flush();
 
 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.

I think we can go down that road, but I'd suggest we push the atomic
patches v8 (with the lastest comments from Joonyoung fixed) and then 
work on the change you are proposing as a follow-up together with the 
other improvements for atomic I already have queued here. This way
we don't take the risk of missing one more merge window.

Gustavo
--
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 v7 12/13] drm/exynos: atomic dpms support

2015-05-27 Thread Inki Dae
Hi Gustavo,

On 2015년 05월 28일 05:27, Gustavo Padovan wrote:
 Hi Inki,
 
 2015-05-27 Inki Dae inki@samsung.com:
 
 Hi Gustavo,

 On 2015년 05월 23일 00:40, Gustavo Padovan wrote:
 From: Gustavo Padovan gustavo.pado...@collabora.co.uk

 Run dpms operations through the atomic intefaces. This basically removes
 the .dpms() callback from econders and crtcs and use .disable() and
 .enable() to turn the crtc on and off.

 v2: Address comments by Joonyoung:
 - make hdmi code call -disable() instead of -dpms()
 - do not use WARN_ON on crtc enable/disable

 v3: - Fix build failure after the hdmi change in v2
 - Change dpms helper of ptn3460 bridge

 Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
 Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  drivers/gpu/drm/bridge/ps8622.c |  2 +-
  drivers/gpu/drm/bridge/ptn3460.c|  2 +-
  drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 
 -
  drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
  drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
  drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
  drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
  10 files changed, 69 insertions(+), 75 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/ps8622.c 
 b/drivers/gpu/drm/bridge/ps8622.c
 index b604326..d686235 100644
 --- a/drivers/gpu/drm/bridge/ps8622.c
 +++ b/drivers/gpu/drm/bridge/ps8622.c
 @@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static const struct drm_connector_funcs ps8622_connector_funcs = {
 -   .dpms = drm_helper_connector_dpms,
 +   .dpms = drm_atomic_helper_connector_dpms,
 .fill_modes = drm_helper_probe_single_connector_modes,
 .detect = ps8622_detect,
 .destroy = ps8622_connector_destroy,
 diff --git a/drivers/gpu/drm/bridge/ptn3460.c 
 b/drivers/gpu/drm/bridge/ptn3460.c
 index 8ed3617..260bc9f 100644
 --- a/drivers/gpu/drm/bridge/ptn3460.c
 +++ b/drivers/gpu/drm/bridge/ptn3460.c
 @@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct 
 drm_connector *connector)
  }
  
  static struct drm_connector_funcs ptn3460_connector_funcs = {
 -   .dpms = drm_helper_connector_dpms,
 +   .dpms = drm_atomic_helper_connector_dpms,
 .fill_modes = drm_helper_probe_single_connector_modes,
 .detect = ptn3460_detect,
 .destroy = ptn3460_connector_destroy,
 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
 index 195fe60..c9995b1 100644
 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
 +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
 @@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
 drm_connector *connector)
  }
  

 [--snip--]

  
  static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 -   .dpms   = exynos_drm_crtc_dpms,
 -   .prepare= exynos_drm_crtc_prepare,
 -   .commit = exynos_drm_crtc_commit,
 +   .enable = exynos_drm_crtc_enable,
 +   .disable= exynos_drm_crtc_disable,
 .mode_fixup = exynos_drm_crtc_mode_fixup,
 .mode_set   = drm_helper_crtc_mode_set,
 .mode_set_nofb  = exynos_drm_crtc_mode_set_nofb,

 I think it'd be better to use atomic_flush callback to enable global dma
 like commit callback did. Is there any reason that you don't use
 atomic_begin and atomic_flush callbacks?

 atomic relevant codes I looked into do as follows,

 atomic_begin();

 atomic_update();  /* this will call win_commit callback to set a overlay
 relevant registers and enable its dma channel. */

 atomic_flush();

 So atomic overlay updating between atomic_begin() ~ atomic_flush() will
 be guaranteed.
 
 I think we can go down that road, but I'd suggest we push the atomic
 patches v8 (with the lastest comments from Joonyoung fixed) and then 
 work on the change you are proposing as a follow-up together with the 
 other improvements for atomic I already have queued here. This way
 we don't take the risk of missing one more merge window.

We(I and Joonyoung) will have discussion about this patch series. For
this, we have already started to analyze entire atomic features
including your patch set so I'd merge it at end of next week according
to the discussion. I'm not kind of sure yet but I will merge it as long
as there is no big problem.

Thanks,
Inki Dae

 
   Gustavo
 --
 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
 

--
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  

[PATCH v7 12/13] drm/exynos: atomic dpms support

2015-05-22 Thread Gustavo Padovan
From: Gustavo Padovan gustavo.pado...@collabora.co.uk

Run dpms operations through the atomic intefaces. This basically removes
the .dpms() callback from econders and crtcs and use .disable() and
.enable() to turn the crtc on and off.

v2: Address comments by Joonyoung:
- make hdmi code call -disable() instead of -dpms()
- do not use WARN_ON on crtc enable/disable

v3: - Fix build failure after the hdmi change in v2
- Change dpms helper of ptn3460 bridge

Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
Reviewed-by: Joonyoung Shim jy0922.s...@samsung.com
Tested-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
---
 drivers/gpu/drm/bridge/ps8622.c |  2 +-
 drivers/gpu/drm/bridge/ptn3460.c|  2 +-
 drivers/gpu/drm/exynos/exynos_dp_core.c |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.c| 95 -
 drivers/gpu/drm/exynos/exynos_drm_dpi.c |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.h |  4 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++--
 drivers/gpu/drm/exynos/exynos_drm_vidi.c|  2 +-
 drivers/gpu/drm/exynos/exynos_hdmi.c|  6 +-
 10 files changed, 69 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ps8622.c b/drivers/gpu/drm/bridge/ps8622.c
index b604326..d686235 100644
--- a/drivers/gpu/drm/bridge/ps8622.c
+++ b/drivers/gpu/drm/bridge/ps8622.c
@@ -499,7 +499,7 @@ static void ps8622_connector_destroy(struct drm_connector 
*connector)
 }
 
 static const struct drm_connector_funcs ps8622_connector_funcs = {
-   .dpms = drm_helper_connector_dpms,
+   .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ps8622_detect,
.destroy = ps8622_connector_destroy,
diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index 8ed3617..260bc9f 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -260,7 +260,7 @@ static void ptn3460_connector_destroy(struct drm_connector 
*connector)
 }
 
 static struct drm_connector_funcs ptn3460_connector_funcs = {
-   .dpms = drm_helper_connector_dpms,
+   .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = ptn3460_detect,
.destroy = ptn3460_connector_destroy,
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 195fe60..c9995b1 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -954,7 +954,7 @@ static void exynos_dp_connector_destroy(struct 
drm_connector *connector)
 }
 
 static struct drm_connector_funcs exynos_dp_connector_funcs = {
-   .dpms = drm_helper_connector_dpms,
+   .dpms = drm_atomic_helper_connector_dpms,
.fill_modes = drm_helper_probe_single_connector_modes,
.detect = exynos_dp_detect,
.destroy = exynos_dp_connector_destroy,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index fd5ef2c..ca501a9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -22,48 +22,54 @@
 #include exynos_drm_encoder.h
 #include exynos_drm_plane.h
 
-static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
+static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+   struct exynos_drm_plane *exynos_plane = to_exynos_plane(crtc-primary);
 
-   DRM_DEBUG_KMS(crtc[%d] mode[%d]\n, crtc-base.id, mode);
-
-   if (exynos_crtc-dpms == mode) {
-   DRM_DEBUG_KMS(desired dpms mode is same as previous one.\n);
+   if (exynos_crtc-enabled)
return;
-   }
-
-   if (mode  DRM_MODE_DPMS_ON) {
-   /* wait for the completion of page flip. */
-   if (!wait_event_timeout(exynos_crtc-pending_flip_queue,
-   (exynos_crtc-event == NULL), HZ/20))
-   exynos_crtc-event = NULL;
-   drm_crtc_vblank_off(crtc);
-   }
 
if (exynos_crtc-ops-dpms)
-   exynos_crtc-ops-dpms(exynos_crtc, mode);
+   exynos_crtc-ops-dpms(exynos_crtc, DRM_MODE_DPMS_ON);
 
-   exynos_crtc-dpms = mode;
+   exynos_crtc-enabled = true;
 
-   if (mode == DRM_MODE_DPMS_ON)
-   drm_crtc_vblank_on(crtc);
-}
+   drm_crtc_vblank_on(crtc);
 
-static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
-{
-   /* drm framework doesn't check NULL. */
+   if (exynos_crtc-ops-win_commit)
+   exynos_crtc-ops-win_commit(exynos_crtc, exynos_plane-zpos);
 }
 
-static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
+static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
 {
struct exynos_drm_crtc *exynos_crtc =