Re: [PATCH] drm/exynos: switch to universal plane API

2014-09-19 Thread Andrzej Hajda 
On 09/19/2014 03:02 AM, Joonyoung Shim wrote:
 Hi Andrzej,

 On 09/18/2014 10:17 PM, Andrzej Hajda wrote:
 The patch replaces legacy functions
 drm_plane_init() / drm_crtc_init() with
 drm_universal_plane_init() and drm_crtc_init_with_planes().
 It allows to replace fake primary plane with the real one.

 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 ---
 Hi Inki,

 I have tested this patch with trats board, it looks OK.
 As a side effect it should solve fb refcounting issues
 reported by me and Daniel Drake.

 Regards
 Andrzej
 ---
  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
  drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
  drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
  4 files changed, 47 insertions(+), 41 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index b68e58f..d2f713e 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -32,7 +32,6 @@ enum exynos_crtc_mode {
   * Exynos specific crtc structure.
   *
   * @drm_crtc: crtc object.
 - * @drm_plane: pointer of private plane object for this crtc
   * @manager: the manager associated with this crtc
   * @pipe: a crtc index created at load() with a new crtc object creation
   *  and the crtc object would be set to private-crtc array
 @@ -46,7 +45,6 @@ enum exynos_crtc_mode {
   */
  struct exynos_drm_crtc {
  struct drm_crtc drm_crtc;
 -struct drm_plane*plane;
  struct exynos_drm_manager   *manager;
  unsigned intpipe;
  unsigned intdpms;
 @@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
  
  exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
  
 -exynos_plane_commit(exynos_crtc-plane);
 +exynos_plane_commit(crtc-primary);
  
  if (manager-ops-commit)
  manager-ops-commit(manager);
  
 -exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
 +exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
  }
  
  static bool
 @@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
 drm_display_mode *mode,
  {
  struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
  struct exynos_drm_manager *manager = exynos_crtc-manager;
 -struct drm_plane *plane = exynos_crtc-plane;
 +struct drm_framebuffer *fb = crtc-primary-fb;
  unsigned int crtc_w;
  unsigned int crtc_h;
 -int ret;
  
  /*
   * copy the mode data adjusted by mode_fixup() into crtc-mode
 @@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
 drm_display_mode *mode,
   */
  memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
  
 -crtc_w = crtc-primary-fb-width - x;
 -crtc_h = crtc-primary-fb-height - y;
 +crtc_w = fb-width - x;
 +crtc_h = fb-height - y;
  
  if (manager-ops-mode_set)
  manager-ops-mode_set(manager, crtc-mode);
  
 -ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -x, y, crtc_w, crtc_h);
 -if (ret)
 -return ret;
 -
 -plane-crtc = crtc;
 -plane-fb = crtc-primary-fb;
 -drm_framebuffer_reference(plane-fb);
 -
 -return 0;
 +return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 + crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  }
  
  static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, 
 int y,
struct drm_framebuffer *old_fb)
  {
  struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 -struct drm_plane *plane = exynos_crtc-plane;
 +struct drm_framebuffer *fb = crtc-primary-fb;
  unsigned int crtc_w;
  unsigned int crtc_h;
  int ret;
 @@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
 drm_crtc *crtc, int x, int y,
  return -EPERM;
  }
  
 -crtc_w = crtc-primary-fb-width - x;
 -crtc_h = crtc-primary-fb-height - y;
 +crtc_w = fb-width - x;
 +crtc_h = fb-height - y;
  
 -ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -x, y, crtc_w, crtc_h);
 +ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  if (ret)
  return ret;
  
 @@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc 
 *crtc)
  
  private-crtc[exynos_crtc-pipe] = NULL;
  
 +crtc-primary-funcs-destroy(crtc-primary);
 This is unnecessary.

The question is how these object should be destroyed. In current code
crtc is destroyed in fimd_unbind and it is called before
drm_mode_config_cleanup
which destroys all planes.
In such case primary plane will stay with .crtc

Re: [PATCH] drm/exynos: switch to universal plane API

2014-09-19 Thread Joonyoung Shim 
Hi,

On 09/19/2014 07:54 PM, Andrzej Hajda wrote:
 On 09/19/2014 03:02 AM, Joonyoung Shim wrote:
 Hi Andrzej,

 On 09/18/2014 10:17 PM, Andrzej Hajda wrote:
 The patch replaces legacy functions
 drm_plane_init() / drm_crtc_init() with
 drm_universal_plane_init() and drm_crtc_init_with_planes().
 It allows to replace fake primary plane with the real one.

 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 ---
 Hi Inki,

 I have tested this patch with trats board, it looks OK.
 As a side effect it should solve fb refcounting issues
 reported by me and Daniel Drake.

 Regards
 Andrzej
 ---
  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
  drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
  drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
  4 files changed, 47 insertions(+), 41 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index b68e58f..d2f713e 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -32,7 +32,6 @@ enum exynos_crtc_mode {
   * Exynos specific crtc structure.
   *
   * @drm_crtc: crtc object.
 - * @drm_plane: pointer of private plane object for this crtc
   * @manager: the manager associated with this crtc
   * @pipe: a crtc index created at load() with a new crtc object creation
   * and the crtc object would be set to private-crtc array
 @@ -46,7 +45,6 @@ enum exynos_crtc_mode {
   */
  struct exynos_drm_crtc {
 struct drm_crtc drm_crtc;
 -   struct drm_plane*plane;
 struct exynos_drm_manager   *manager;
 unsigned intpipe;
 unsigned intdpms;
 @@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc 
 *crtc)
  
 exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
  
 -   exynos_plane_commit(exynos_crtc-plane);
 +   exynos_plane_commit(crtc-primary);
  
 if (manager-ops-commit)
 manager-ops-commit(manager);
  
 -   exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
 +   exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
  }
  
  static bool
 @@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
 drm_display_mode *mode,
  {
 struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 struct exynos_drm_manager *manager = exynos_crtc-manager;
 -   struct drm_plane *plane = exynos_crtc-plane;
 +   struct drm_framebuffer *fb = crtc-primary-fb;
 unsigned int crtc_w;
 unsigned int crtc_h;
 -   int ret;
  
 /*
  * copy the mode data adjusted by mode_fixup() into crtc-mode
 @@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, 
 struct drm_display_mode *mode,
  */
 memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
  
 -   crtc_w = crtc-primary-fb-width - x;
 -   crtc_h = crtc-primary-fb-height - y;
 +   crtc_w = fb-width - x;
 +   crtc_h = fb-height - y;
  
 if (manager-ops-mode_set)
 manager-ops-mode_set(manager, crtc-mode);
  
 -   ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -   x, y, crtc_w, crtc_h);
 -   if (ret)
 -   return ret;
 -
 -   plane-crtc = crtc;
 -   plane-fb = crtc-primary-fb;
 -   drm_framebuffer_reference(plane-fb);
 -
 -   return 0;
 +   return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  }
  
  static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, 
 int y,
   struct drm_framebuffer *old_fb)
  {
 struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 -   struct drm_plane *plane = exynos_crtc-plane;
 +   struct drm_framebuffer *fb = crtc-primary-fb;
 unsigned int crtc_w;
 unsigned int crtc_h;
 int ret;
 @@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
 drm_crtc *crtc, int x, int y,
 return -EPERM;
 }
  
 -   crtc_w = crtc-primary-fb-width - x;
 -   crtc_h = crtc-primary-fb-height - y;
 +   crtc_w = fb-width - x;
 +   crtc_h = fb-height - y;
  
 -   ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -   x, y, crtc_w, crtc_h);
 +   ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
 if (ret)
 return ret;
  
 @@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc 
 *crtc)
  
 private-crtc[exynos_crtc-pipe] = NULL;
  
 +   crtc-primary-funcs-destroy(crtc-primary);
 This is unnecessary.
 
 The question is how these object should be destroyed. In current code
 crtc is destroyed in fimd_unbind and it is called before
 drm_mode_config_cleanup
 which destroys all planes.
 In such case primary plane will stay with .crtc

Re: [PATCH] drm/exynos: switch to universal plane API

2014-09-19 Thread Joonyoung Shim 
Hi,

On 09/19/2014 08:11 PM, Joonyoung Shim wrote:
 Hi,
 
 On 09/19/2014 07:54 PM, Andrzej Hajda wrote:
 On 09/19/2014 03:02 AM, Joonyoung Shim wrote:
 Hi Andrzej,

 On 09/18/2014 10:17 PM, Andrzej Hajda wrote:
 The patch replaces legacy functions
 drm_plane_init() / drm_crtc_init() with
 drm_universal_plane_init() and drm_crtc_init_with_planes().
 It allows to replace fake primary plane with the real one.

 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 ---
 Hi Inki,

 I have tested this patch with trats board, it looks OK.
 As a side effect it should solve fb refcounting issues
 reported by me and Daniel Drake.

 Regards
 Andrzej
 ---
  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
  drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
  drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
  4 files changed, 47 insertions(+), 41 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index b68e58f..d2f713e 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -32,7 +32,6 @@ enum exynos_crtc_mode {
   * Exynos specific crtc structure.
   *
   * @drm_crtc: crtc object.
 - * @drm_plane: pointer of private plane object for this crtc
   * @manager: the manager associated with this crtc
   * @pipe: a crtc index created at load() with a new crtc object creation
   *and the crtc object would be set to private-crtc array
 @@ -46,7 +45,6 @@ enum exynos_crtc_mode {
   */
  struct exynos_drm_crtc {
struct drm_crtc drm_crtc;
 -  struct drm_plane*plane;
struct exynos_drm_manager   *manager;
unsigned intpipe;
unsigned intdpms;
 @@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc 
 *crtc)
  
exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
  
 -  exynos_plane_commit(exynos_crtc-plane);
 +  exynos_plane_commit(crtc-primary);
  
if (manager-ops-commit)
manager-ops-commit(manager);
  
 -  exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
 +  exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
  }
  
  static bool
 @@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, 
 struct drm_display_mode *mode,
  {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct exynos_drm_manager *manager = exynos_crtc-manager;
 -  struct drm_plane *plane = exynos_crtc-plane;
 +  struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
 -  int ret;
  
/*
 * copy the mode data adjusted by mode_fixup() into crtc-mode
 @@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, 
 struct drm_display_mode *mode,
 */
memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
  
 -  crtc_w = crtc-primary-fb-width - x;
 -  crtc_h = crtc-primary-fb-height - y;
 +  crtc_w = fb-width - x;
 +  crtc_h = fb-height - y;
  
if (manager-ops-mode_set)
manager-ops-mode_set(manager, crtc-mode);
  
 -  ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -  x, y, crtc_w, crtc_h);
 -  if (ret)
 -  return ret;
 -
 -  plane-crtc = crtc;
 -  plane-fb = crtc-primary-fb;
 -  drm_framebuffer_reference(plane-fb);
 -
 -  return 0;
 +  return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  }
  
  static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, 
 int y,
  struct drm_framebuffer *old_fb)
  {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 -  struct drm_plane *plane = exynos_crtc-plane;
 +  struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
int ret;
 @@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
 drm_crtc *crtc, int x, int y,
return -EPERM;
}
  
 -  crtc_w = crtc-primary-fb-width - x;
 -  crtc_h = crtc-primary-fb-height - y;
 +  crtc_w = fb-width - x;
 +  crtc_h = fb-height - y;
  
 -  ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -  x, y, crtc_w, crtc_h);
 +  ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +  crtc_w, crtc_h, x, y, crtc_w, crtc_h);
if (ret)
return ret;
  
 @@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc 
 *crtc)
  
private-crtc[exynos_crtc-pipe] = NULL;
  
 +  crtc-primary-funcs-destroy(crtc-primary);
 This is unnecessary.

 The question is how these object should be destroyed. In current code
 crtc is destroyed in fimd_unbind and it is called before
 drm_mode_config_cleanup
 which destroys all planes.
 In such case primary plane will stay with .crtc

Re: [PATCH] drm/exynos: switch to universal plane API

2014-09-19 Thread Andrzej Hajda 
On 09/19/2014 01:11 PM, Joonyoung Shim wrote:
 Hi,

 On 09/19/2014 07:54 PM, Andrzej Hajda wrote:
 On 09/19/2014 03:02 AM, Joonyoung Shim wrote:
 Hi Andrzej,

 On 09/18/2014 10:17 PM, Andrzej Hajda wrote:
 The patch replaces legacy functions
 drm_plane_init() / drm_crtc_init() with
 drm_universal_plane_init() and drm_crtc_init_with_planes().
 It allows to replace fake primary plane with the real one.

 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 ---
 Hi Inki,

 I have tested this patch with trats board, it looks OK.
 As a side effect it should solve fb refcounting issues
 reported by me and Daniel Drake.

 Regards
 Andrzej
 ---
  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
  drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
  drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
  4 files changed, 47 insertions(+), 41 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index b68e58f..d2f713e 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -32,7 +32,6 @@ enum exynos_crtc_mode {
   * Exynos specific crtc structure.
   *
   * @drm_crtc: crtc object.
 - * @drm_plane: pointer of private plane object for this crtc
   * @manager: the manager associated with this crtc
   * @pipe: a crtc index created at load() with a new crtc object creation
   *and the crtc object would be set to private-crtc array
 @@ -46,7 +45,6 @@ enum exynos_crtc_mode {
   */
  struct exynos_drm_crtc {
struct drm_crtc drm_crtc;
 -  struct drm_plane*plane;
struct exynos_drm_manager   *manager;
unsigned intpipe;
unsigned intdpms;
 @@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc 
 *crtc)
  
exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
  
 -  exynos_plane_commit(exynos_crtc-plane);
 +  exynos_plane_commit(crtc-primary);
  
if (manager-ops-commit)
manager-ops-commit(manager);
  
 -  exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
 +  exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
  }
  
  static bool
 @@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, 
 struct drm_display_mode *mode,
  {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct exynos_drm_manager *manager = exynos_crtc-manager;
 -  struct drm_plane *plane = exynos_crtc-plane;
 +  struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
 -  int ret;
  
/*
 * copy the mode data adjusted by mode_fixup() into crtc-mode
 @@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, 
 struct drm_display_mode *mode,
 */
memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
  
 -  crtc_w = crtc-primary-fb-width - x;
 -  crtc_h = crtc-primary-fb-height - y;
 +  crtc_w = fb-width - x;
 +  crtc_h = fb-height - y;
  
if (manager-ops-mode_set)
manager-ops-mode_set(manager, crtc-mode);
  
 -  ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -  x, y, crtc_w, crtc_h);
 -  if (ret)
 -  return ret;
 -
 -  plane-crtc = crtc;
 -  plane-fb = crtc-primary-fb;
 -  drm_framebuffer_reference(plane-fb);
 -
 -  return 0;
 +  return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  }
  
  static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, 
 int y,
  struct drm_framebuffer *old_fb)
  {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 -  struct drm_plane *plane = exynos_crtc-plane;
 +  struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
int ret;
 @@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
 drm_crtc *crtc, int x, int y,
return -EPERM;
}
  
 -  crtc_w = crtc-primary-fb-width - x;
 -  crtc_h = crtc-primary-fb-height - y;
 +  crtc_w = fb-width - x;
 +  crtc_h = fb-height - y;
  
 -  ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 -  x, y, crtc_w, crtc_h);
 +  ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +  crtc_w, crtc_h, x, y, crtc_w, crtc_h);
if (ret)
return ret;
  
 @@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc 
 *crtc)
  
private-crtc[exynos_crtc-pipe] = NULL;
  
 +  crtc-primary-funcs-destroy(crtc-primary);
 This is unnecessary.
 The question is how these object should be destroyed. In current code
 crtc is destroyed in fimd_unbind and it is called before
 drm_mode_config_cleanup
 which destroys all planes.
 In such case primary plane will stay with .crtc pointing to

[PATCH] drm/exynos: switch to universal plane API

2014-09-18 Thread Andrzej Hajda 
The patch replaces legacy functions
drm_plane_init() / drm_crtc_init() with
drm_universal_plane_init() and drm_crtc_init_with_planes().
It allows to replace fake primary plane with the real one.

Signed-off-by: Andrzej Hajda a.ha...@samsung.com
---
Hi Inki,

I have tested this patch with trats board, it looks OK.
As a side effect it should solve fb refcounting issues
reported by me and Daniel Drake.

Regards
Andrzej
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 ---
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
 drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
 4 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index b68e58f..d2f713e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -32,7 +32,6 @@ enum exynos_crtc_mode {
  * Exynos specific crtc structure.
  *
  * @drm_crtc: crtc object.
- * @drm_plane: pointer of private plane object for this crtc
  * @manager: the manager associated with this crtc
  * @pipe: a crtc index created at load() with a new crtc object creation
  * and the crtc object would be set to private-crtc array
@@ -46,7 +45,6 @@ enum exynos_crtc_mode {
  */
 struct exynos_drm_crtc {
struct drm_crtc drm_crtc;
-   struct drm_plane*plane;
struct exynos_drm_manager   *manager;
unsigned intpipe;
unsigned intdpms;
@@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
 
exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
 
-   exynos_plane_commit(exynos_crtc-plane);
+   exynos_plane_commit(crtc-primary);
 
if (manager-ops-commit)
manager-ops-commit(manager);
 
-   exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
+   exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
 }
 
 static bool
@@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
drm_display_mode *mode,
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
struct exynos_drm_manager *manager = exynos_crtc-manager;
-   struct drm_plane *plane = exynos_crtc-plane;
+   struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
-   int ret;
 
/*
 * copy the mode data adjusted by mode_fixup() into crtc-mode
@@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
drm_display_mode *mode,
 */
memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
 
-   crtc_w = crtc-primary-fb-width - x;
-   crtc_h = crtc-primary-fb-height - y;
+   crtc_w = fb-width - x;
+   crtc_h = fb-height - y;
 
if (manager-ops-mode_set)
manager-ops-mode_set(manager, crtc-mode);
 
-   ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
crtc_w, crtc_h,
-   x, y, crtc_w, crtc_h);
-   if (ret)
-   return ret;
-
-   plane-crtc = crtc;
-   plane-fb = crtc-primary-fb;
-   drm_framebuffer_reference(plane-fb);
-
-   return 0;
+   return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
+crtc_w, crtc_h, x, y, crtc_w, crtc_h);
 }
 
 static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, int y,
  struct drm_framebuffer *old_fb)
 {
struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-   struct drm_plane *plane = exynos_crtc-plane;
+   struct drm_framebuffer *fb = crtc-primary-fb;
unsigned int crtc_w;
unsigned int crtc_h;
int ret;
@@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
drm_crtc *crtc, int x, int y,
return -EPERM;
}
 
-   crtc_w = crtc-primary-fb-width - x;
-   crtc_h = crtc-primary-fb-height - y;
+   crtc_w = fb-width - x;
+   crtc_h = fb-height - y;
 
-   ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
crtc_w, crtc_h,
-   x, y, crtc_w, crtc_h);
+   ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
+   crtc_w, crtc_h, x, y, crtc_w, crtc_h);
if (ret)
return ret;
 
@@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc *crtc)
 
private-crtc[exynos_crtc-pipe] = NULL;
 
+   crtc-primary-funcs-destroy(crtc-primary);
drm_crtc_cleanup(crtc);
kfree(exynos_crtc);
 }
@@ -304,8 +294,7 @@ static int exynos_drm_crtc_set_property(struct drm_crtc 
*crtc,
exynos_drm_crtc_commit(crtc);
break;
case CRTC_MODE_BLANK:
-

Re: [PATCH] drm/exynos: switch to universal plane API

2014-09-18 Thread Joonyoung Shim 
Hi Andrzej,

On 09/18/2014 10:17 PM, Andrzej Hajda wrote:
 The patch replaces legacy functions
 drm_plane_init() / drm_crtc_init() with
 drm_universal_plane_init() and drm_crtc_init_with_planes().
 It allows to replace fake primary plane with the real one.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 ---
 Hi Inki,
 
 I have tested this patch with trats board, it looks OK.
 As a side effect it should solve fb refcounting issues
 reported by me and Daniel Drake.
 
 Regards
 Andrzej
 ---
  drivers/gpu/drm/exynos/exynos_drm_crtc.c  | 63 
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c   |  5 ++-
  drivers/gpu/drm/exynos/exynos_drm_plane.c | 17 +
  drivers/gpu/drm/exynos/exynos_drm_plane.h |  3 +-
  4 files changed, 47 insertions(+), 41 deletions(-)
 
 diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c 
 b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 index b68e58f..d2f713e 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
 @@ -32,7 +32,6 @@ enum exynos_crtc_mode {
   * Exynos specific crtc structure.
   *
   * @drm_crtc: crtc object.
 - * @drm_plane: pointer of private plane object for this crtc
   * @manager: the manager associated with this crtc
   * @pipe: a crtc index created at load() with a new crtc object creation
   *   and the crtc object would be set to private-crtc array
 @@ -46,7 +45,6 @@ enum exynos_crtc_mode {
   */
  struct exynos_drm_crtc {
   struct drm_crtc drm_crtc;
 - struct drm_plane*plane;
   struct exynos_drm_manager   *manager;
   unsigned intpipe;
   unsigned intdpms;
 @@ -94,12 +92,12 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
  
   exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
  
 - exynos_plane_commit(exynos_crtc-plane);
 + exynos_plane_commit(crtc-primary);
  
   if (manager-ops-commit)
   manager-ops-commit(manager);
  
 - exynos_plane_dpms(exynos_crtc-plane, DRM_MODE_DPMS_ON);
 + exynos_plane_dpms(crtc-primary, DRM_MODE_DPMS_ON);
  }
  
  static bool
 @@ -123,10 +121,9 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
 drm_display_mode *mode,
  {
   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
   struct exynos_drm_manager *manager = exynos_crtc-manager;
 - struct drm_plane *plane = exynos_crtc-plane;
 + struct drm_framebuffer *fb = crtc-primary-fb;
   unsigned int crtc_w;
   unsigned int crtc_h;
 - int ret;
  
   /*
* copy the mode data adjusted by mode_fixup() into crtc-mode
 @@ -134,29 +131,21 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct 
 drm_display_mode *mode,
*/
   memcpy(crtc-mode, adjusted_mode, sizeof(*adjusted_mode));
  
 - crtc_w = crtc-primary-fb-width - x;
 - crtc_h = crtc-primary-fb-height - y;
 + crtc_w = fb-width - x;
 + crtc_h = fb-height - y;
  
   if (manager-ops-mode_set)
   manager-ops-mode_set(manager, crtc-mode);
  
 - ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 - x, y, crtc_w, crtc_h);
 - if (ret)
 - return ret;
 -
 - plane-crtc = crtc;
 - plane-fb = crtc-primary-fb;
 - drm_framebuffer_reference(plane-fb);
 -
 - return 0;
 + return exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 +  crtc_w, crtc_h, x, y, crtc_w, crtc_h);
  }
  
  static int exynos_drm_crtc_mode_set_commit(struct drm_crtc *crtc, int x, int 
 y,
 struct drm_framebuffer *old_fb)
  {
   struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
 - struct drm_plane *plane = exynos_crtc-plane;
 + struct drm_framebuffer *fb = crtc-primary-fb;
   unsigned int crtc_w;
   unsigned int crtc_h;
   int ret;
 @@ -167,11 +156,11 @@ static int exynos_drm_crtc_mode_set_commit(struct 
 drm_crtc *crtc, int x, int y,
   return -EPERM;
   }
  
 - crtc_w = crtc-primary-fb-width - x;
 - crtc_h = crtc-primary-fb-height - y;
 + crtc_w = fb-width - x;
 + crtc_h = fb-height - y;
  
 - ret = exynos_plane_mode_set(plane, crtc, crtc-primary-fb, 0, 0, 
 crtc_w, crtc_h,
 - x, y, crtc_w, crtc_h);
 + ret = exynos_plane_mode_set(crtc-primary, crtc, fb, 0, 0,
 + crtc_w, crtc_h, x, y, crtc_w, crtc_h);
   if (ret)
   return ret;
  
 @@ -279,6 +268,7 @@ static void exynos_drm_crtc_destroy(struct drm_crtc *crtc)
  
   private-crtc[exynos_crtc-pipe] = NULL;
  
 + crtc-primary-funcs-destroy(crtc-primary);

This is unnecessary.

   drm_crtc_cleanup(crtc);
   kfree(exynos_crtc);
  }
 @@ -304,8 +294,7 @@ static int exynos_drm_crtc_set_property(struct drm_crtc 
 *crtc,
   exynos_drm_crtc_commit(crtc);