| Message ID | 1355396719-25286-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org > [mailto:dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org] On > Behalf Of Daniel Vetter > Sent: Thursday, December 13, 2012 8:05 PM > To: DRI Development > Cc: Nouveau Dev; Intel Graphics Development; Daniel Vetter > Subject: [PATCH 2/2] drm/exynos: reorder framebuffer init sequence > > For user framebuffers it's easier to just inline the > exynos_drm_framebuffer_init helper instead of trying to adjust it - > most of the things that helper sets up need to be overwritten anyway > again due to the multiple backing storage objects support exynos has, > but does not use for the fbdev. > Hi Daniel, I'd rebase this patch to -next. This patch is conflicted with -next. And if there is no any problem after test, will apply it. Thanks, Inki Dae > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/exynos/exynos_drm_fb.c | 32 ++++++++++++++++++++-------- > ---- > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c > b/drivers/gpu/drm/exynos/exynos_drm_fb.c > index 4ef4cd3..aea6500 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c > @@ -136,15 +136,15 @@ exynos_drm_framebuffer_init(struct drm_device *dev, > return ERR_PTR(-ENOMEM); > } > > + drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); > + exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj); > + > ret = drm_framebuffer_init(dev, &exynos_fb->fb, > &exynos_drm_fb_funcs); > if (ret) { > DRM_ERROR("failed to initialize framebuffer\n"); > return ERR_PTR(ret); > } > > - drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); > - exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj); > - > return &exynos_fb->fb; > } > > @@ -190,9 +190,8 @@ exynos_user_fb_create(struct drm_device *dev, struct > drm_file *file_priv, > struct drm_mode_fb_cmd2 *mode_cmd) > { > struct drm_gem_object *obj; > - struct drm_framebuffer *fb; > struct exynos_drm_fb *exynos_fb; > - int i; > + int i, ret; > > DRM_DEBUG_KMS("%s\n", __FILE__); > > @@ -202,13 +201,13 @@ exynos_user_fb_create(struct drm_device *dev, struct > drm_file *file_priv, > return ERR_PTR(-ENOENT); > } > > - fb = exynos_drm_framebuffer_init(dev, mode_cmd, obj); > - if (IS_ERR(fb)) { > - drm_gem_object_unreference_unlocked(obj); > - return fb; > + exynos_fb = kzalloc(sizeof(*exynos_fb), GFP_KERNEL); > + if (!exynos_fb) { > + DRM_ERROR("failed to allocate exynos drm framebuffer\n"); > + return ERR_PTR(-ENOMEM); > } > > - exynos_fb = to_exynos_fb(fb); > + drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); > exynos_fb->buf_cnt = exynos_drm_format_num_buffers(mode_cmd); > > DRM_DEBUG_KMS("buf_cnt = %d\n", exynos_fb->buf_cnt); > @@ -218,14 +217,23 @@ exynos_user_fb_create(struct drm_device *dev, struct > drm_file *file_priv, > mode_cmd->handles[i]); > if (!obj) { > DRM_ERROR("failed to lookup gem object\n"); > - exynos_drm_fb_destroy(fb); > + kfree(exynos_fb); > return ERR_PTR(-ENOENT); > } > > exynos_fb->exynos_gem_obj[i] = to_exynos_gem_obj(obj); > } > > - return fb; > + ret = drm_framebuffer_init(dev, &exynos_fb->fb, > &exynos_drm_fb_funcs); > + if (ret) { > + for (i = 0; i < exynos_fb->buf_cnt; i++) > + drm_gem_object_unreference_unlocked(&exynos_fb- > >exynos_gem_obj[i]->base); > + > + kfree(exynos_fb); > + return ERR_PTR(ret); > + } > + > + return &exynos_fb->fb; > } > > struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer > *fb, > -- > 1.7.10.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Inki, I've pushed out the latest bits to http://cgit.freedesktop.org/~danvet/drm/log/?h=drm-kms-locking with some hacks on top to be able to compile all the arm drivers. Testing feedback of the entire pile would be awesome, especially since you've had some issues with framebuffer lifecycle and those should now be correctly fixable with the proper refcounting. If you have too many conflicts pls yell so that I can include your base into mine and rebase the entire series. Thanks, Daniel On Thu, Dec 13, 2012 at 1:26 PM, Inki Dae <inki.dae@samsung.com> wrote: >> -----Original Message----- >> From: dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org >> [mailto:dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org] On >> Behalf Of Daniel Vetter >> Sent: Thursday, December 13, 2012 8:05 PM >> To: DRI Development >> Cc: Nouveau Dev; Intel Graphics Development; Daniel Vetter >> Subject: [PATCH 2/2] drm/exynos: reorder framebuffer init sequence >> >> For user framebuffers it's easier to just inline the >> exynos_drm_framebuffer_init helper instead of trying to adjust it - >> most of the things that helper sets up need to be overwritten anyway >> again due to the multiple backing storage objects support exynos has, >> but does not use for the fbdev. >> > > Hi Daniel, > > I'd rebase this patch to -next. This patch is conflicted with -next. > And if there is no any problem after test, will apply it.
2012/12/13 Daniel Vetter <daniel.vetter@ffwll.ch> > Hi Inki, > > I've pushed out the latest bits to > http://cgit.freedesktop.org/~danvet/drm/log/?h=drm-kms-locking with > some hacks on top to be able to compile all the arm drivers. Testing > feedback of the entire pile would be awesome, especially since you've > had some issues with framebuffer lifecycle and those should now be > correctly fixable with the proper refcounting. If you have too many > conflicts pls yell so that I can include your base into mine and > rebase the entire series. > > Hi Daniel, How about rebasing this patch to top of git:// people.freedesktop.org/~airlied/linux.git drm-next? Exynos's many patches have already been merged to drm-next. Or if you are ok, I'd like to rebase your patch to -next and test it. I don't care either way. :) If there is any problem, please let me know. Thanks, Inki Dae Thanks, Daniel > > On Thu, Dec 13, 2012 at 1:26 PM, Inki Dae <inki.dae@samsung.com> wrote: > >> -----Original Message----- > >> From: dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org > >> [mailto:dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org] > On > >> Behalf Of Daniel Vetter > >> Sent: Thursday, December 13, 2012 8:05 PM > >> To: DRI Development > >> Cc: Nouveau Dev; Intel Graphics Development; Daniel Vetter > >> Subject: [PATCH 2/2] drm/exynos: reorder framebuffer init sequence > >> > >> For user framebuffers it's easier to just inline the > >> exynos_drm_framebuffer_init helper instead of trying to adjust it - > >> most of the things that helper sets up need to be overwritten anyway > >> again due to the multiple backing storage objects support exynos has, > >> but does not use for the fbdev. > >> > > > > Hi Daniel, > > > > I'd rebase this patch to -next. This patch is conflicted with -next. > > And if there is no any problem after test, will apply it. > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Thu, Dec 13, 2012 at 4:16 PM, Inki Dae <inki.dae@samsung.com> wrote: > How about rebasing this patch to top of > git://people.freedesktop.org/~airlied/linux.git drm-next? > Exynos's many patches have already been merged to drm-next. Or if you are > ok, I'd like to rebase your patch to -next and test it. I don't care either > way. :) I've pushed out a rebased version, but somehow I can't compile-test exynos any more :( Probably missing some other stuff from linux-next. Anyway, I don't mind which patch you pick or whether you just roll your own, the important thing is that drm_framebuffer_init is called _after_ everything is set up and initialized. Everything else doesn't really matter. -Daniel
2012/12/14 Daniel Vetter <daniel.vetter@ffwll.ch> > On Thu, Dec 13, 2012 at 4:16 PM, Inki Dae <inki.dae@samsung.com> wrote: > > How about rebasing this patch to top of > > git://people.freedesktop.org/~airlied/linux.git drm-next? > > Exynos's many patches have already been merged to drm-next. Or if you are > > ok, I'd like to rebase your patch to -next and test it. I don't care > either > > way. :) > > I've pushed out a rebased version, but somehow I can't compile-test > exynos any more :( Probably missing some other stuff from linux-next. > Anyway, I don't mind which patch you pick or whether you just roll > your own, the important thing is that drm_framebuffer_init is called > _after_ everything is set up and initialized. Everything else doesn't > really matter. > I've rebase your patch to -next and tested it. But your patch had null pointer issue to gem object so I fixed it. For this, you can refer to the below link, http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git;a=shortlog;h=refs/heads/exynos-drm-next And I gonna request git pull to -next including that patch today. If there is any problem, please let me know. Thanks, Inki Dae. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Fri, Dec 14, 2012 at 5:57 AM, Inki Dae <inki.dae@samsung.com> wrote: > 2012/12/14 Daniel Vetter <daniel.vetter@ffwll.ch> >> >> On Thu, Dec 13, 2012 at 4:16 PM, Inki Dae <inki.dae@samsung.com> wrote: >> > How about rebasing this patch to top of >> > git://people.freedesktop.org/~airlied/linux.git drm-next? >> > Exynos's many patches have already been merged to drm-next. Or if you >> > are >> > ok, I'd like to rebase your patch to -next and test it. I don't care >> > either >> > way. :) >> >> I've pushed out a rebased version, but somehow I can't compile-test >> exynos any more :( Probably missing some other stuff from linux-next. >> Anyway, I don't mind which patch you pick or whether you just roll >> your own, the important thing is that drm_framebuffer_init is called >> _after_ everything is set up and initialized. Everything else doesn't >> really matter. > > > I've rebase your patch to -next and tested it. But your patch had null > pointer issue to gem object so I fixed it. > For this, you can refer to the below link, > > http://git.kernel.org/?p=linux/kernel/git/daeinki/drm-exynos.git;a=shortlog;h=refs/heads/exynos-drm-next > > And I gonna request git pull to -next including that patch today. If there > is any problem, please let me know. Oops, sorry for missing the gem obj assignment, your updated version looks good. Thanks for fixing it up. -Daniel
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 4ef4cd3..aea6500 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c @@ -136,15 +136,15 @@ exynos_drm_framebuffer_init(struct drm_device *dev, return ERR_PTR(-ENOMEM); } + drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); + exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj); + ret = drm_framebuffer_init(dev, &exynos_fb->fb, &exynos_drm_fb_funcs); if (ret) { DRM_ERROR("failed to initialize framebuffer\n"); return ERR_PTR(ret); } - drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); - exynos_fb->exynos_gem_obj[0] = to_exynos_gem_obj(obj); - return &exynos_fb->fb; } @@ -190,9 +190,8 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, struct drm_mode_fb_cmd2 *mode_cmd) { struct drm_gem_object *obj; - struct drm_framebuffer *fb; struct exynos_drm_fb *exynos_fb; - int i; + int i, ret; DRM_DEBUG_KMS("%s\n", __FILE__); @@ -202,13 +201,13 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, return ERR_PTR(-ENOENT); } - fb = exynos_drm_framebuffer_init(dev, mode_cmd, obj); - if (IS_ERR(fb)) { - drm_gem_object_unreference_unlocked(obj); - return fb; + exynos_fb = kzalloc(sizeof(*exynos_fb), GFP_KERNEL); + if (!exynos_fb) { + DRM_ERROR("failed to allocate exynos drm framebuffer\n"); + return ERR_PTR(-ENOMEM); } - exynos_fb = to_exynos_fb(fb); + drm_helper_mode_fill_fb_struct(&exynos_fb->fb, mode_cmd); exynos_fb->buf_cnt = exynos_drm_format_num_buffers(mode_cmd); DRM_DEBUG_KMS("buf_cnt = %d\n", exynos_fb->buf_cnt); @@ -218,14 +217,23 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, mode_cmd->handles[i]); if (!obj) { DRM_ERROR("failed to lookup gem object\n"); - exynos_drm_fb_destroy(fb); + kfree(exynos_fb); return ERR_PTR(-ENOENT); } exynos_fb->exynos_gem_obj[i] = to_exynos_gem_obj(obj); } - return fb; + ret = drm_framebuffer_init(dev, &exynos_fb->fb, &exynos_drm_fb_funcs); + if (ret) { + for (i = 0; i < exynos_fb->buf_cnt; i++) + drm_gem_object_unreference_unlocked(&exynos_fb->exynos_gem_obj[i]->base); + + kfree(exynos_fb); + return ERR_PTR(ret); + } + + return &exynos_fb->fb; } struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer *fb,
For user framebuffers it's easier to just inline the exynos_drm_framebuffer_init helper instead of trying to adjust it - most of the things that helper sets up need to be overwritten anyway again due to the multiple backing storage objects support exynos has, but does not use for the fbdev. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/exynos/exynos_drm_fb.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)