Re: [Intel-gfx] [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Hi, 21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글: > Once EDID is parsed, the monitor HDMI support information is available > through drm_display_info.is_hdmi. Retriving the same information with > drm_detect_hdmi_monitor() is less efficient. Change to > drm_display_info.is_hdmi > > Signed-off-by: Claudio Suarez > --- > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions( > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 7655142a4651..a563d6386abe 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector > *connector) > if (!edid) > return -ENODEV; > > - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); > + /* This updates connector->display_info */ > + drm_connector_update_edid_property(connector, edid); > + > + hdata->dvi_mode = !connector->display_info.is_hdmi; Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi parsed from EDID. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/drm_edid.c?h=v5.14.14#n4725 Signed-off-by: Inki Dae Thanks, Inki Dae > DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", > (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), > edid->width_cm, edid->height_cm); > > - drm_connector_update_edid_property(connector, edid); > cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid); > > ret = drm_add_edid_modes(connector, edid); >
Re: [Intel-gfx] [PATCH 03/12] drm/exynos: Don't set allow_fb_modifiers explicitly
21. 4. 13. 오후 6:48에 Daniel Vetter 이(가) 쓴 글: > Since > > commit 890880ddfdbe256083170866e49c87618b706ac7 > Author: Paul Kocialkowski > Date: Fri Jan 4 09:56:10 2019 +0100 > > drm: Auto-set allow_fb_modifiers when given modifiers at plane init > > this is done automatically as part of plane init, if drivers set the > modifier list correctly. Which is the case here. > > Signed-off-by: Daniel Vetter Acked-by: Inki Dae Thanks, Inki Dae > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Cc: Krzysztof Kozlowski > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-samsung-...@vger.kernel.org > --- > drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c > b/drivers/gpu/drm/exynos/exynos_drm_fb.c > index 64370b634cca..79fa3649185c 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c > @@ -177,7 +177,5 @@ void exynos_drm_mode_config_init(struct drm_device *dev) > dev->mode_config.funcs = &exynos_drm_mode_config_funcs; > dev->mode_config.helper_private = &exynos_drm_mode_config_helpers; > > - dev->mode_config.allow_fb_modifiers = true; > - > dev->mode_config.normalize_zpos = true; > } > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/21] drm/exynos: Use drm_fb_helper_fill_info
19. 3. 26. 오후 10:19에 Daniel Vetter 이(가) 쓴 글: > This will give the exynos fbdev a name! > > v2: Rebase > > Signed-off-by: Daniel Vetter Acked-by: Inki Dae Thanks, Inki Dae > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Cc: Kukjin Kim > Cc: Krzysztof Kozlowski > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-samsung-...@vger.kernel.org > --- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > index c30dd88cdb25..581a6a207995 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > @@ -87,11 +87,9 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper > *helper, > return PTR_ERR(fbi); > } > > - fbi->par = helper; > fbi->fbops = &exynos_drm_fb_ops; > > - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth); > - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height); > + drm_fb_helper_fill_info(fbi, helper, sizes); > > nr_pages = exynos_gem->size >> PAGE_SHIFT; > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13] drm/exynos: Provide ddc symlink in connector's sysfs
19. 8. 1. 오전 1:58에 Andrzej Pietrasiewicz 이(가) 쓴 글:
> Switch to using the ddc provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz
> Acked-by: Sam Ravnborg
> Reviewed-by: Emil Velikov
Acked-by: Inki Dae
Thanks,
Inki Dae
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index bc1565f1822a..d4a9c9e17436 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -940,8 +940,10 @@ static int hdmi_create_connector(struct drm_encoder
> *encoder)
> connector->interlace_allowed = true;
> connector->polled = DRM_CONNECTOR_POLL_HPD;
>
> - ret = drm_connector_init(hdata->drm_dev, connector,
> - &hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA);
> + ret = drm_connector_init_with_ddc(hdata->drm_dev, connector,
> + &hdmi_connector_funcs,
> + DRM_MODE_CONNECTOR_HDMIA,
> + hdata->ddc_adpt);
> if (ret) {
> DRM_DEV_ERROR(hdata->dev,
> "Failed to initialize connector with drm\n");
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/7] drm/exynos: Use drm_encoder_mask()
20. 2. 12. 오전 1:22에 Ville Syrjala 이(가) 쓴 글: > From: Ville Syrjälä > > Replace the hand rolled encoder bitmask thing with drm_encoder_mask() > > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Acked-by: Thomas Zimmermann > Signed-off-by: Ville Syrjälä Acked-by: Inki Dae THanks, Inki Dae > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index ba0f868b2477..57defeb44522 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -270,7 +270,7 @@ static int exynos_drm_bind(struct device *dev) > struct drm_encoder *encoder; > struct drm_device *drm; > unsigned int clone_mask; > - int cnt, ret; > + int ret; > > drm = drm_dev_alloc(&exynos_drm_driver, dev); > if (IS_ERR(drm)) > @@ -293,10 +293,9 @@ static int exynos_drm_bind(struct device *dev) > exynos_drm_mode_config_init(drm); > > /* setup possible_clones. */ > - cnt = 0; > clone_mask = 0; > list_for_each_entry(encoder, &drm->mode_config.encoder_list, head) > - clone_mask |= (1 << (cnt++)); > + clone_mask |= drm_encoder_mask(encoder); > > list_for_each_entry(encoder, &drm->mode_config.encoder_list, head) > encoder->possible_clones = clone_mask; > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 04/11] drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()
2017년 12월 06일 03:24에 Noralf Trønnes 이(가) 쓴 글:
> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
> It can also use drm_fb_helper_output_poll_changed() as its
> .output_poll_changed callback.
>
> Cc: Inki Dae
> Cc: Joonyoung Shim
> Cc: Seung-Woo Kim
> Cc: Kyungmin Park
> Signed-off-by: Noralf Trønnes
> Acked-by: Daniel Vetter
Seems you missed my ACK,
http://www.spinics.net/lists/intel-gfx/msg146188.html
Thanks,
Inki Dae
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++--
> drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 --
> drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 --
> 4 files changed, 3 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 82b72425a42f..2f2bd6e37e62 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev,
> struct drm_file *file)
> file->driver_priv = NULL;
> }
>
> -static void exynos_drm_lastclose(struct drm_device *dev)
> -{
> - exynos_drm_fbdev_restore_mode(dev);
> -}
> -
> static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
> .fault = exynos_drm_gem_fault,
> .open = drm_gem_vm_open,
> @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = {
> .driver_features= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
> | DRIVER_ATOMIC | DRIVER_RENDER,
> .open = exynos_drm_open,
> - .lastclose = exynos_drm_lastclose,
> + .lastclose = drm_fb_helper_lastclose,
> .postclose = exynos_drm_postclose,
> .gem_free_object_unlocked = exynos_drm_gem_free_object,
> .gem_vm_ops = &exynos_drm_gem_vm_ops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 8208df56a88f..0faaf829f5bf 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs
> exynos_drm_mode_config_helpers = {
>
> static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
> .fb_create = exynos_user_fb_create,
> - .output_poll_changed = exynos_drm_output_poll_changed,
> + .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = exynos_atomic_check,
> .atomic_commit = drm_atomic_helper_commit,
> };
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index dfb66ecf417b..132dd52d0ac7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev)
> private->fb_helper = NULL;
> }
>
> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
> -{
> - struct exynos_drm_private *private = dev->dev_private;
> -
> - if (!private || !private->fb_helper)
> - return;
> -
> - drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
> -}
> -
> -void exynos_drm_output_poll_changed(struct drm_device *dev)
> -{
> - struct exynos_drm_private *private = dev->dev_private;
> - struct drm_fb_helper *fb_helper = private->fb_helper;
> -
> - drm_fb_helper_hotplug_event(fb_helper);
> -}
> -
> void exynos_drm_fbdev_suspend(struct drm_device *dev)
> {
> struct exynos_drm_private *private = dev->dev_private;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> index 645d1bb7f665..b33847223a85 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.h
> @@ -19,8 +19,6 @@
>
> int exynos_drm_fbdev_init(struct drm_device *dev);
> void exynos_drm_fbdev_fini(struct drm_device *dev);
> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev);
> -void exynos_drm_output_poll_changed(struct drm_device *dev);
> void exynos_drm_fbdev_suspend(struct drm_device *drm);
> void exynos_drm_fbdev_resume(struct drm_device *drm);
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/26] drm/exynos: Use for_each_crtc_in_state
2016-05-30 3:35 GMT+09:00 Daniel Vetter :
> We want to hide drm_atomic_state internals better.
>
Acked-by: Inki Dae
> Cc: Inki Dae
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 2dd820e23b0c..cabc5fd0246d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -267,6 +267,8 @@ int exynos_atomic_commit(struct drm_device *dev, struct
> drm_atomic_state *state,
> {
> struct exynos_drm_private *priv = dev->dev_private;
> struct exynos_atomic_commit *commit;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> int i, ret;
>
> commit = kzalloc(sizeof(*commit), GFP_KERNEL);
> @@ -288,10 +290,8 @@ int exynos_atomic_commit(struct drm_device *dev, struct
> drm_atomic_state *state,
> /* Wait until all affected CRTCs have completed previous commits and
> * mark them as pending.
> */
> - for (i = 0; i < dev->mode_config.num_crtc; ++i) {
> - if (state->crtcs[i])
> - commit->crtcs |= 1 << drm_crtc_index(state->crtcs[i]);
> - }
> + for_each_crtc_in_state(state, crtc, crtc_state, i)
> + commit->crtcs |= 1 << drm_crtc_index(crtc);
>
> wait_event(priv->wait, !commit_is_pending(priv, commit->crtcs));
>
> --
> 2.8.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/9] drm/fb-helper: Push down modeset lock into FB helpers
Hi Thierry,
2016년 06월 08일 00:26에 Thierry Reding 이(가) 쓴 글:
> From: Thierry Reding
>
> Move the modeset locking from drivers into FB helpers.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/drm_fb_helper.c| 59
> +-
> drivers/gpu/drm/i915/intel_dp_mst.c| 4 ---
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 7
> 3 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9afd4208596f..252c7557bdb5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -95,8 +95,8 @@ static LIST_HEAD(kernel_fb_helper_list);
> * mmap page writes.
> */
>
> -int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> - struct drm_connector *connector)
> +static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> {
> struct drm_fb_helper_connector **temp;
> struct drm_fb_helper_connector *conn;
> @@ -129,6 +129,20 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper
> *fb_helper,
> fb_helper->connector_info[fb_helper->connector_count++] = conn;
> return 0;
> }
> +
> +int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
> + struct drm_connector *connector)
> +{
> + int err;
> +
> + mutex_lock(&fb_helper->dev->mode_config.mutex);
> +
> + err = __drm_fb_helper_add_one_connector(fb_helper, connector);
I think you don't need to make __drm_fb_helper_add_on_connector function but
just you can implement the codes here instead. And the use of prefix '__' thing
would not good.
And my concern about this is that other drivers may need to call this function
after taking a same mutex lock. Can you assure anyone not to call this function
with mutex lock?
If there is such case then,
some_function()
{
mutex_lock();
...
mutex_unlock();
drm_fb_helper_add_one_connector();
...
}
So in this case, other users should consider to make sure to unlock before
calling this function. That would be now really what you want.
Thanks,
Inki Dae
> +
> + mutex_unlock(&fb_helper->dev->mode_config.mutex);
> +
> + return err;
> +}
> EXPORT_SYMBOL(drm_fb_helper_add_one_connector);
>
> /**
> @@ -155,28 +169,28 @@ int drm_fb_helper_single_add_all_connectors(struct
> drm_fb_helper *fb_helper)
> return 0;
>
> mutex_lock(&dev->mode_config.mutex);
> +
> drm_for_each_connector(connector, dev) {
> - ret = drm_fb_helper_add_one_connector(fb_helper, connector);
> + ret = __drm_fb_helper_add_one_connector(fb_helper, connector);
> + if (ret) {
> + for (i = 0; i < fb_helper->connector_count; i++) {
> + kfree(fb_helper->connector_info[i]);
> + fb_helper->connector_info[i] = NULL;
> + }
I think all resources should be released by someone who allocated the resources
in case of failure. I mean that if some routine of
__drm_fb_helper_add_on_connector function failed than the function make sure to
release the resources allocated. I know this is really not what you did but
original code did. So how about cleanning up this thing?
Thanks,
Inki Dae
>
> - if (ret)
> - goto fail;
> - }
> - mutex_unlock(&dev->mode_config.mutex);
> - return 0;
> -fail:
> - for (i = 0; i < fb_helper->connector_count; i++) {
> - kfree(fb_helper->connector_info[i]);
> - fb_helper->connector_info[i] = NULL;
> + fb_helper->connector_count = 0;
> + break;
> + }
> }
> - fb_helper->connector_count = 0;
> +
> mutex_unlock(&dev->mode_config.mutex);
>
> return ret;
> }
> EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>
> -int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> -struct drm_connector *connector)
> +static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper
> *fb_helper,
> + struct drm_connector *connector)
> {
> struct drm_fb_helper_connector *fb_helper_connector;
> int i, j;
> @@ -193,6 +207,7 @@ int drm_fb_helper_remove_one_connector(struct
> drm_fb_helper *fb_helper,
>
> if (i == fb_helper->
Re: [Intel-gfx] [PATCH 07/26] drm/exynos: Use for_each_crtc_in_state
2016년 05월 30일 03:35에 Daniel Vetter 이(가) 쓴 글:
> We want to hide drm_atomic_state internals better.
Acked-by: Inki Dae
Thanks,
Inki Dae
>
> Cc: Inki Dae
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 2dd820e23b0c..cabc5fd0246d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -267,6 +267,8 @@ int exynos_atomic_commit(struct drm_device *dev, struct
> drm_atomic_state *state,
> {
> struct exynos_drm_private *priv = dev->dev_private;
> struct exynos_atomic_commit *commit;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> int i, ret;
>
> commit = kzalloc(sizeof(*commit), GFP_KERNEL);
> @@ -288,10 +290,8 @@ int exynos_atomic_commit(struct drm_device *dev, struct
> drm_atomic_state *state,
> /* Wait until all affected CRTCs have completed previous commits and
>* mark them as pending.
>*/
> - for (i = 0; i < dev->mode_config.num_crtc; ++i) {
> - if (state->crtcs[i])
> - commit->crtcs |= 1 << drm_crtc_index(state->crtcs[i]);
> - }
> + for_each_crtc_in_state(state, crtc, crtc_state, i)
> + commit->crtcs |= 1 << drm_crtc_index(crtc);
>
> wait_event(priv->wait, !commit_is_pending(priv, commit->crtcs));
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
>> Rendering operations to the dma-buf are tracked implicitly via the
>> reservation_object (dmabuf->resv). This is used to allow poll() to
>> wait upon outstanding rendering (or just query the current status of
>> rendering). The dma-buf sync ioctl allows userspace to prepare the
>> dma-buf for CPU access, which should include waiting upon rendering.
>> (Some drivers may need to do more work to ensure that the dma-buf mmap
>> is coherent as well as complete.)
>>
>> v2: Always wait upon the reservation object implicitly. We choose to do
>> it after the native handler in case it can do so more efficiently.
>>
>> Testcase: igt/prime_vgem
>> Testcase: igt/gem_concurrent_blit # *vgem*
>> Signed-off-by: Chris Wilson
>> Cc: Sumit Semwal
>> Cc: Daniel Vetter
>> Cc: Eric Anholt
>> Cc: linux-me...@vger.kernel.org
>> Cc: dri-de...@lists.freedesktop.org
>> Cc: linaro-mm-...@lists.linaro.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>> drivers/dma-buf/dma-buf.c | 23 +++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index ddaee60ae52a..cf04d249a6a4 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment
>> *attach,
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
>>
>> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> + enum dma_data_direction direction)
>> +{
>> +bool write = (direction == DMA_BIDIRECTIONAL ||
>> + direction == DMA_TO_DEVICE);
>> +struct reservation_object *resv = dmabuf->resv;
>> +long ret;
>> +
>> +/* Wait on any implicit rendering fences */
>> +ret = reservation_object_wait_timeout_rcu(resv, write, true,
>> + MAX_SCHEDULE_TIMEOUT);
>> +if (ret < 0)
>> +return ret;
>> +
>> +return 0;
>> +}
>>
>> /**
>> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf
>> from the
>> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>> if (dmabuf->ops->begin_cpu_access)
>> ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>>
>> +/* Ensure that all fences are waited upon - but we first allow
>> + * the native handler the chance to do so more efficiently if it
>> + * chooses. A double invocation here will be reasonably cheap no-op.
>> + */
>> +if (ret == 0)
>> +ret = __dma_buf_begin_cpu_access(dmabuf, direction);
>
> Not sure we should wait first and the flush or the other way round. But I
> don't think it'll matter for any current dma-buf exporter, so meh.
>
Sorry for late comment. I wonder there is no problem in case that GPU or other
DMA device tries to access this dma buffer after dma_buf_begin_cpu_access call.
I think in this case, they - GPU or DMA devices - would make a mess of the dma
buffer while CPU is accessing the buffer.
This patch is in mainline already so if this is real problem then I think we
sould choose,
1. revert this patch from mainline
2. make sure to prevent other DMA devices to try to access the buffer while CPU
is accessing the buffer.
Thanks.
> Reviewed-by: Daniel Vetter
>
> Sumits, can you pls pick this one up and put into drm-misc?
> -Daniel
>
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>> --
>> 2.8.1
>>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/exynos: Nuke dummy fb->dirty callback
Hi Daniel,
2016년 04월 27일 20:38에 Daniel Vetter 이(가) 쓴 글:
> It's an optional hook. Might be needed for frontbuffer rendering on
> manual upload displays, but a simple TODO doesn't explain at all what
> needs to be done or why.
We have a plan for partial update support but not now yet. Picked it up.
Thanks,
Inki Dae
>
> Cc: Inki Dae
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index 81cc5537cf25..f851a40ac6cb 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -97,20 +97,9 @@ static int exynos_drm_fb_create_handle(struct
> drm_framebuffer *fb,
>&exynos_fb->exynos_gem[0]->base, handle);
> }
>
> -static int exynos_drm_fb_dirty(struct drm_framebuffer *fb,
> - struct drm_file *file_priv, unsigned flags,
> - unsigned color, struct drm_clip_rect *clips,
> - unsigned num_clips)
> -{
> - /* TODO */
> -
> - return 0;
> -}
> -
> static const struct drm_framebuffer_funcs exynos_drm_fb_funcs = {
> .destroy= exynos_drm_fb_destroy,
> .create_handle = exynos_drm_fb_create_handle,
> - .dirty = exynos_drm_fb_dirty,
> };
>
> struct drm_framebuffer *
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/35] drm/exynos: Use lockless gem BO free callback
2016년 04월 27일 02:29에 Daniel Vetter 이(가) 쓴 글:
> No dev->struct_mutex anywhere to be seen.
Acked-by: Inki Dae
Thanks,
Inki Dae
>
> Cc: Inki Dae
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 5344940c8a07..f534ed62065e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -418,7 +418,7 @@ static struct drm_driver exynos_drm_driver = {
> .get_vblank_counter = drm_vblank_no_hw_counter,
> .enable_vblank = exynos_drm_crtc_enable_vblank,
> .disable_vblank = exynos_drm_crtc_disable_vblank,
> - .gem_free_object= exynos_drm_gem_free_object,
> + .gem_free_object_unlocked = exynos_drm_gem_free_object,
> .gem_vm_ops = &exynos_drm_gem_vm_ops,
> .dumb_create= exynos_drm_gem_dumb_create,
> .dumb_map_offset= exynos_drm_gem_dumb_map_offset,
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/22] drm/exynos: Remove event cancelling from postclose
Hi Daniel,
It seems your patch is exactly same as below my one I posted before,
http://www.spinics.net/lists/dri-devel/msg97922.html
Anyway, it's ok if this patch can go to mainline.
Acked-by: Inki Dae
2016년 01월 12일 06:41에 Daniel Vetter 이(가) 쓴 글:
> The core takes care of this now. And since kfree(NULL) is ok we can
> simplify the function even further now.
>
> Cc: Inki Dae
> Acked-by: Daniel Stone
> Reviewed-by: Alex Deucher
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9756797a15a5..868ab9f54f17 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -335,20 +335,6 @@ static void exynos_drm_preclose(struct drm_device *dev,
>
> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file
> *file)
> {
> - struct drm_pending_event *e, *et;
> - unsigned long flags;
> -
> - if (!file->driver_priv)
> - return;
> -
> - spin_lock_irqsave(&dev->event_lock, flags);
> - /* Release all events handled by page flip handler but not freed. */
> - list_for_each_entry_safe(e, et, &file->event_list, link) {
> - list_del(&e->link);
> - e->destroy(e);
> - }
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
> kfree(file->driver_priv);
> file->driver_priv = NULL;
> }
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/37] drm/exynos: Drop drm_vblank_cleanup
Hi Daniel, 2017년 05월 24일 23:51에 Daniel Vetter 이(가) 쓴 글: > Only in the load failure path, where the hardware is quiet anyway. > > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 50294a7bd29d..1c814b9342af 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -376,7 +376,7 @@ static int exynos_drm_bind(struct device *dev) > /* Probe non kms sub drivers and virtual display driver. */ > ret = exynos_drm_device_subdrv_probe(drm); > if (ret) > - goto err_cleanup_vblank; > + goto err_unbind_all; With this change shouldn't you post the patch to remove drm_vblank_init and setup vblank stuff in drm_crtc_init together? I couldn't find the relevant patch on your patch series[1]. As of now, I think resource leak would happen with this patch only. Thanks, Inki Dae [1] http://www.spinics.net/lists/dri-devel/msg142387.html > > drm_mode_config_reset(drm); > > @@ -407,8 +407,6 @@ static int exynos_drm_bind(struct device *dev) > exynos_drm_fbdev_fini(drm); > drm_kms_helper_poll_fini(drm); > exynos_drm_device_subdrv_remove(drm); > -err_cleanup_vblank: > - drm_vblank_cleanup(drm); > err_unbind_all: > component_unbind_all(drm->dev, drm); > err_mode_config_cleanup: > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/37] drm/exynos: Drop drm_vblank_cleanup
2017년 05월 31일 17:45에 Daniel Vetter 이(가) 쓴 글: > On Tue, May 30, 2017 at 09:03:34AM +0900, Inki Dae wrote: >> Hi Daniel, >> >> 2017년 05월 24일 23:51에 Daniel Vetter 이(가) 쓴 글: >>> Only in the load failure path, where the hardware is quiet anyway. >>> >>> Cc: Inki Dae >>> Cc: Joonyoung Shim >>> Cc: Seung-Woo Kim >>> Cc: Kyungmin Park >>> Signed-off-by: Daniel Vetter >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> index 50294a7bd29d..1c814b9342af 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> @@ -376,7 +376,7 @@ static int exynos_drm_bind(struct device *dev) >>> /* Probe non kms sub drivers and virtual display driver. */ >>> ret = exynos_drm_device_subdrv_probe(drm); >>> if (ret) >>> - goto err_cleanup_vblank; >>> + goto err_unbind_all; >> >> With this change shouldn't you post the patch to remove drm_vblank_init and >> setup vblank stuff in drm_crtc_init together? >> I couldn't find the relevant patch on your patch series[1]. > > No, drm_vblank_cleanup is already called in the core. The only reason to > call it in the driver is to fall back from kms to ums when irq setup > somehow failed, then you need to disable vblank support again. > > The only driver which ever needed this was radeon, and radeon long ago > lost ums support. drm_vblank_cleanup is already called for you, and most > drivers don't even do this cleanup call. But somehow a lot of people > copied from radeon without understanding what it does. > > Looking at the last patch in this series might help a bit in understanding > the history here. Can you pls reevaluate the patch? Thanks for explaination. Confirmed. Reviewed-by: Inki Dae Thanks, Inki Dae > > Thanks, Daniel > >> As of now, I think resource leak would happen with this patch only. >> >> Thanks, >> Inki Dae >> >> [1] http://www.spinics.net/lists/dri-devel/msg142387.html >> >>> >>> drm_mode_config_reset(drm); >>> >>> @@ -407,8 +407,6 @@ static int exynos_drm_bind(struct device *dev) >>> exynos_drm_fbdev_fini(drm); >>> drm_kms_helper_poll_fini(drm); >>> exynos_drm_device_subdrv_remove(drm); >>> -err_cleanup_vblank: >>> - drm_vblank_cleanup(drm); >>> err_unbind_all: >>> component_unbind_all(drm->dev, drm); >>> err_mode_config_cleanup: >>> > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/13] drm/exynos: Remove custom FB helper deferred setup
2017년 07월 05일 00:18에 Daniel Vetter 이(가) 쓴 글:
> From: Thierry Reding
>
> The FB helper core now supports deferred setup, so the driver's custom
> implementation can be removed.
Reviewed-by: Inki Dae
Tested-by: Inki Dae
Thanks,
Inki Dae
>
> v2: Drop NULL check, drm_fb_helper_hotplug_event handles that already.
>
> Cc: Inki Dae
> Cc: Joonyoung Shim
> Cc: Seung-Woo Kim
> Cc: Kyungmin Park
> Signed-off-by: Thierry Reding (v1)
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 --
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 26 +-
> 2 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 35a8dfc93836..cab9e12d7846 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -395,8 +395,9 @@ static int exynos_drm_bind(struct device *dev)
> /* init kms poll for handling hpd */
> drm_kms_helper_poll_init(drm);
>
> - /* force connectors detection */
> - drm_helper_hpd_irq_event(drm);
> + ret = exynos_drm_fbdev_init(drm);
> + if (ret)
> + goto err_cleanup_poll;
>
> /* register the DRM device */
> ret = drm_dev_register(drm, 0);
> @@ -407,6 +408,7 @@ static int exynos_drm_bind(struct device *dev)
>
> err_cleanup_fbdev:
> exynos_drm_fbdev_fini(drm);
> +err_cleanup_poll:
> drm_kms_helper_poll_fini(drm);
> exynos_drm_device_subdrv_remove(drm);
> err_unbind_all:
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 641531243e04..c3a068409b48 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -183,24 +183,6 @@ static const struct drm_fb_helper_funcs
> exynos_drm_fb_helper_funcs = {
> .fb_probe = exynos_drm_fbdev_create,
> };
>
> -static bool exynos_drm_fbdev_is_anything_connected(struct drm_device *dev)
> -{
> - struct drm_connector *connector;
> - bool ret = false;
> -
> - mutex_lock(&dev->mode_config.mutex);
> - list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> - if (connector->status != connector_status_connected)
> - continue;
> -
> - ret = true;
> - break;
> - }
> - mutex_unlock(&dev->mode_config.mutex);
> -
> - return ret;
> -}
> -
> int exynos_drm_fbdev_init(struct drm_device *dev)
> {
> struct exynos_drm_fbdev *fbdev;
> @@ -211,9 +193,6 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
> if (!dev->mode_config.num_crtc || !dev->mode_config.num_connector)
> return 0;
>
> - if (!exynos_drm_fbdev_is_anything_connected(dev))
> - return 0;
> -
> fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
> if (!fbdev)
> return -ENOMEM;
> @@ -304,8 +283,5 @@ void exynos_drm_output_poll_changed(struct drm_device
> *dev)
> struct exynos_drm_private *private = dev->dev_private;
> struct drm_fb_helper *fb_helper = private->fb_helper;
>
> - if (fb_helper)
> - drm_fb_helper_hotplug_event(fb_helper);
> - else
> - exynos_drm_fbdev_init(dev);
> + drm_fb_helper_hotplug_event(fb_helper);
> }
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 06/15] drm/exynos: Use drm_fb_helper_lastclose() and _poll_changed()
2017년 10월 31일 19:28에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Oct 30, 2017 at 04:39:42PM +0100, Noralf Trønnes wrote:
>> This driver can use drm_fb_helper_lastclose() as its .lastclose callback.
>> It can also use drm_fb_helper_output_poll_changed() as its
>> .output_poll_changed callback.
>>
>> Cc: Inki Dae
>> Cc: Joonyoung Shim
>> Cc: Seung-Woo Kim
>> Cc: Kyungmin Park
>> Signed-off-by: Noralf Trønnes
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 ++--
>> drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +-
>> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 18 --
>> drivers/gpu/drm/exynos/exynos_drm_fbdev.h | 2 --
>> 4 files changed, 3 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index e651a58c18cf..70f4895ac49c 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -16,6 +16,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -89,11 +90,6 @@ static void exynos_drm_postclose(struct drm_device *dev,
>> struct drm_file *file)
>> file->driver_priv = NULL;
>> }
>>
>> -static void exynos_drm_lastclose(struct drm_device *dev)
>> -{
>> -exynos_drm_fbdev_restore_mode(dev);
>> -}
>> -
>> static const struct vm_operations_struct exynos_drm_gem_vm_ops = {
>> .fault = exynos_drm_gem_fault,
>> .open = drm_gem_vm_open,
>> @@ -140,7 +136,7 @@ static struct drm_driver exynos_drm_driver = {
>> .driver_features= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME
>>| DRIVER_ATOMIC | DRIVER_RENDER,
>> .open = exynos_drm_open,
>> -.lastclose = exynos_drm_lastclose,
>> +.lastclose = drm_fb_helper_lastclose,
>> .postclose = exynos_drm_postclose,
>> .gem_free_object_unlocked = exynos_drm_gem_free_object,
>> .gem_vm_ops = &exynos_drm_gem_vm_ops,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> index 8208df56a88f..0faaf829f5bf 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
>> @@ -205,7 +205,7 @@ static struct drm_mode_config_helper_funcs
>> exynos_drm_mode_config_helpers = {
>>
>> static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
>> .fb_create = exynos_user_fb_create,
>> -.output_poll_changed = exynos_drm_output_poll_changed,
>> +.output_poll_changed = drm_fb_helper_output_poll_changed,
>> .atomic_check = exynos_atomic_check,
>> .atomic_commit = drm_atomic_helper_commit,
>> };
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> index dfb66ecf417b..132dd52d0ac7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
>> @@ -270,24 +270,6 @@ void exynos_drm_fbdev_fini(struct drm_device *dev)
>> private->fb_helper = NULL;
>> }
>>
>> -void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
>> -{
>> -struct exynos_drm_private *private = dev->dev_private;
>> -
>> -if (!private || !private->fb_helper)
>
> Not sure this isn't risky, exynos has a strange load sequence ... Probably
> best if we get an ack from Inki.
I didn't test this patch on real hardware due to below two issues,
1. many warning messages printed out at vblank period.
- to finalize atomic flush drm_crtc_arm_vblank_event function is called but
Exynos drm driver has no implementation of get_vblank_timestamp and
get_scanout_position callbacks.
2. tranferring Panel commands to Panel device - s6e3ha2 Panel device - timed
out when exynos_drm_fbdev_restore_mode is called.
So I just looked into this patch and looks good to me.
Only a difference between old and new helper functions is above condition -
checking if private and private->fb_helper are null or not.
And dev->dev_private and private->fb_helper are cleared after calling
drm_dev_unregister function which calls drm_fb_helpaer_lastclose function so it
must be no problem.
Acked-by: Inki Dae
Thanks,
Inki Dae
> -Daniel
>> -return;
>> -
>> -drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper);
>> -}
>> -
>> -void exynos_drm_output_poll_changed(struct drm_device *dev)
>
Re: [Intel-gfx] [PATCH 20/24] drm/exynos: Merge pre/postclose hooks
2017년 03월 14일 22:28에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Mar 13, 2017 at 03:18:05PM -0400, Sean Paul wrote:
>> On Wed, Mar 08, 2017 at 03:12:53PM +0100, Daniel Vetter wrote:
>>> Again no apparent explanation for the split except hysterical raisins.
>>> Merging them also makes it a bit more obviuos what's going on wrt the
>>> runtime pm refdancing.
>>
>> Commit message copypasta.
>
> Hm, yeah. Inki, can you pls adjust that when merging to exynos-next? Just
> drop the last sentence that talks about pm refdancing.
> -Daniel
No problem. :)
Thanks,
Inki Dae
>
>>
>> The code is
>>
>> Reviewed-by: Sean Paul
>>
>>>
>>> Cc: Inki Dae
>>> Cc: Joonyoung Shim
>>> Cc: Seung-Woo Kim
>>> Cc: Kyungmin Park
>>> Signed-off-by: Daniel Vetter
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 +---
>>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index b4522f67b3cb..180e3c9884e5 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -194,14 +194,9 @@ static int exynos_drm_open(struct drm_device *dev,
>>> struct drm_file *file)
>>> return ret;
>>> }
>>>
>>> -static void exynos_drm_preclose(struct drm_device *dev,
>>> - struct drm_file *file)
>>> -{
>>> - exynos_drm_subdrv_close(dev, file);
>>> -}
>>> -
>>> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file
>>> *file)
>>> {
>>> + exynos_drm_subdrv_close(dev, file);
>>> kfree(file->driver_priv);
>>> file->driver_priv = NULL;
>>> }
>>> @@ -259,7 +254,6 @@ static struct drm_driver exynos_drm_driver = {
>>> .load = exynos_drm_load,
>>> .unload = exynos_drm_unload,
>>> .open = exynos_drm_open,
>>> - .preclose = exynos_drm_preclose,
>>> .lastclose = exynos_drm_lastclose,
>>> .postclose = exynos_drm_postclose,
>>> .gem_free_object_unlocked = exynos_drm_gem_free_object,
>>> --
>>> 2.11.0
>>>
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/20] drm/gem: create drm_gem_dumb_destroy
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
> Sent: Tuesday, July 16, 2013 4:12 PM
> To: DRI Development
> Cc: Daniel Vetter; Inki Dae; Laurent Pinchart; Intel Graphics Development;
> Ben Skeggs; Rob Clark; Alex Deucher
> Subject: [PATCH 13/20] drm/gem: create drm_gem_dumb_destroy
>
> All the gem based kms drivers really want the same function to
> destroy a dumb framebuffer backing storage object.
>
> So give it to them and roll it out in all drivers.
>
> This still leaves the option open for kms drivers which don't use GEM
> for backing storage, but it does decently simplify matters for gem
> drivers.
>
> Cc: Inki Dae
> Cc: Laurent Pinchart
> Cc: Intel Graphics Development
> Cc: Ben Skeggs
> Cc: Rob Clark
> Cc: Alex Deucher
> Signed-off-by: Daniel Vetter
Acked-by: Inki Dae
Thanks,
Inki Dae
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/ast/ast_drv.h | 3 ---
> drivers/gpu/drm/ast/ast_main.c| 7 ---
> drivers/gpu/drm/cirrus/cirrus_drv.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_drv.h | 3 ---
> drivers/gpu/drm/cirrus/cirrus_main.c | 7 ---
> drivers/gpu/drm/drm_gem.c | 14 ++
> drivers/gpu/drm/drm_gem_cma_helper.c | 10 --
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 20
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 9 -
> drivers/gpu/drm/gma500/gem.c | 17 -
> drivers/gpu/drm/gma500/psb_drv.c | 2 +-
> drivers/gpu/drm/gma500/psb_drv.h | 2 --
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 --
> drivers/gpu/drm/i915/i915_gem.c | 7 ---
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_drv.h | 3 ---
> drivers/gpu/drm/mgag200/mgag200_main.c| 7 ---
> drivers/gpu/drm/nouveau/nouveau_display.c | 7 ---
> drivers/gpu/drm/nouveau/nouveau_display.h | 2 --
> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.c| 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.h| 2 --
> drivers/gpu/drm/omapdrm/omap_gem.c| 15 ---
> drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
> drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
> drivers/gpu/drm/qxl/qxl_dumb.c| 7 ---
> drivers/gpu/drm/radeon/radeon.h | 3 ---
> drivers/gpu/drm/radeon/radeon_drv.c | 5 +
> drivers/gpu/drm/radeon/radeon_gem.c | 7 ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> drivers/gpu/drm/shmobile/shmob_drm_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.h | 2 --
> drivers/gpu/drm/udl/udl_gem.c | 6 --
> drivers/gpu/host1x/drm/drm.c | 2 +-
> drivers/gpu/host1x/drm/gem.c | 6 --
> drivers/gpu/host1x/drm/gem.h | 2 --
> drivers/staging/imx-drm/imx-drm-core.c| 2 +-
> include/drm/drmP.h| 3 +++
> include/drm/drm_gem_cma_helper.h | 8
> 44 files changed, 33 insertions(+), 186 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index df0d0a0..a144fb0 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -216,7 +216,7 @@ static struct drm_driver driver = {
> .gem_free_object = ast_gem_free_object,
> .dumb_create = ast_dumb_create,
> .dumb_map_offset = ast_dumb_mmap_offset,
> - .dumb_destroy = ast_dumb_destroy,
> + .dumb_destroy = drm_gem_dumb_destroy,
>
> };
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 622d4ae..796dbb2 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -322,9 +322,6 @@ ast_bo(struct ttm_buffer_object *bo)
> extern int ast_dumb_create(struct drm_file *file,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
> -extern int ast_dumb_destroy(struct drm_file *file,
> - struct drm_device *dev,
> - uint32_t handle);
>
> extern int ast_gem_init_object(struct drm_gem_object *obj);
> extern void ast_gem_free_object(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index f60fd7b..0ef4228 100644
> --- a/
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
> Sent: Wednesday, August 07, 2013 6:15 PM
> To: DRI Development
> Cc: Intel Graphics Development; Daniel Vetter; Inki Dae
> Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos
> drivers
>
> Note that this is slightly tricky since both drivers store their
> native objects in dma_buf->priv. But both also embed the base
> drm_gem_object at the first position, so the implicit cast is ok.
>
> To use the release helper we need to export it, too.
Yeah, may I repost this patch with additional work? We also need to export
with a gem object instead of specific one like you did.
Thanks,
Inki Dae
>
> Cc: Inki Dae
> Cc: Intel Graphics Development
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_prime.c| 3 ++-
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 23 +--
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 13 +
> include/drm/drmP.h | 1 +
> 4 files changed, 5 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 85e450e..a35f206 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -192,7 +192,7 @@ static void drm_gem_unmap_dma_buf(struct
> dma_buf_attachment *attach,
> /* nothing to be done here */
> }
>
> -static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
> +void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
> {
> struct drm_gem_object *obj = dma_buf->priv;
>
> @@ -202,6 +202,7 @@ static void drm_gem_dmabuf_release(struct dma_buf
> *dma_buf)
> drm_gem_object_unreference_unlocked(obj);
> }
> }
> +EXPORT_SYMBOL(drm_gem_dmabuf_release);
>
> static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
> {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> index a0f997e..3cd56e1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> @@ -127,27 +127,6 @@ static void exynos_gem_unmap_dma_buf(struct
> dma_buf_attachment *attach,
> /* Nothing to do. */
> }
>
> -static void exynos_dmabuf_release(struct dma_buf *dmabuf)
> -{
> - struct exynos_drm_gem_obj *exynos_gem_obj = dmabuf->priv;
> -
> - /*
> - * exynos_dmabuf_release() call means that file object's
> - * f_count is 0 and it calls drm_gem_object_handle_unreference()
> - * to drop the references that these values had been increased
> - * at drm_prime_handle_to_fd()
> - */
> - if (exynos_gem_obj->base.export_dma_buf == dmabuf) {
> - exynos_gem_obj->base.export_dma_buf = NULL;
> -
> - /*
> - * drop this gem object refcount to release allocated buffer
> - * and resources.
> - */
> - drm_gem_object_unreference_unlocked(&exynos_gem_obj->base);
> - }
> -}
> -
> static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
> unsigned long page_num)
> {
> @@ -193,7 +172,7 @@ static struct dma_buf_ops exynos_dmabuf_ops = {
> .kunmap = exynos_gem_dmabuf_kunmap,
> .kunmap_atomic = exynos_gem_dmabuf_kunmap_atomic,
> .mmap = exynos_gem_dmabuf_mmap,
> - .release= exynos_dmabuf_release,
> + .release= drm_gem_dmabuf_release,
> };
>
> struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index f2e185c..63ee1a9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -90,17 +90,6 @@ static void i915_gem_unmap_dma_buf(struct
> dma_buf_attachment *attachment,
> kfree(sg);
> }
>
> -static void i915_gem_dmabuf_release(struct dma_buf *dma_buf)
> -{
> - struct drm_i915_gem_object *obj = dma_buf->priv;
> -
> - if (obj->base.export_dma_buf == dma_buf) {
> - /* drop the reference on the export fd holds */
> - obj->base.export_dma_buf = NULL;
> - drm_gem_object_unreference_unlocked(&obj->base);
> - }
> -}
> -
> static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
> {
> struct drm_i915_gem_object *obj = dma_buf->priv;
> @@ -211,7 +200,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf
> *dma_buf, size_t start, size
> static const struct dma
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
2013/8/7 Daniel Vetter > On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: > > On 08/07/2013 06:55 PM, Daniel Vetter wrote: > > >On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae wrote: > > >>>-Original Message- > > >>>From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] > > >>>Sent: Wednesday, August 07, 2013 6:15 PM > > >>>To: DRI Development > > >>>Cc: Intel Graphics Development; Daniel Vetter; Inki Dae > > >>>Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in > i915/exynos > > >>>drivers > > >>> > > >>>Note that this is slightly tricky since both drivers store their > > >>>native objects in dma_buf->priv. But both also embed the base > > >>>drm_gem_object at the first position, so the implicit cast is ok. > > >>> > > >>>To use the release helper we need to export it, too. > > >>Yeah, may I repost this patch with additional work? We also need to > export > > >>with a gem object instead of specific one like you did. > > > > I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. > > Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common > > drm_gem_dmabuf with low-level hook functions to use prime helpers. > > Ah, but that can easily be done on top of this, right? > Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf->priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. 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-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
2013/8/7 Daniel Vetter > On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae wrote: > > > > > > 2013/8/7 Daniel Vetter > >> > >> On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: > >> > On 08/07/2013 06:55 PM, Daniel Vetter wrote: > >> > >On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae > wrote: > >> > >>>-Original Message- > >> > >>>From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] > >> > >>>Sent: Wednesday, August 07, 2013 6:15 PM > >> > >>>To: DRI Development > >> > >>>Cc: Intel Graphics Development; Daniel Vetter; Inki Dae > >> > >>>Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in > >> > >>> i915/exynos > >> > >>>drivers > >> > >>> > >> > >>>Note that this is slightly tricky since both drivers store their > >> > >>>native objects in dma_buf->priv. But both also embed the base > >> > >>>drm_gem_object at the first position, so the implicit cast is ok. > >> > >>> > >> > >>>To use the release helper we need to export it, too. > >> > >>Yeah, may I repost this patch with additional work? We also need to > >> > >> export > >> > >>with a gem object instead of specific one like you did. > >> > > >> > I think dmabuf stuff of exynos can be replaced to common > drm_gem_dmabuf. > >> > Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common > >> > drm_gem_dmabuf with low-level hook functions to use prime helpers. > >> > >> Ah, but that can easily be done on top of this, right? > > > > > > Daniel, could you remove exynos related codes from your patch set? Your > > patch set would make exynos broken because you didn't consider exporting > > with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store > base > > gem object in dma_buf->priv. So I think your patch set is not complete > set, > > and That is why exynos needs the additional work I mentioned above. So I > > just wanted to repost your patch set + new one. > > Nope, my patch should not break exynos since the base gem_object is > the first member of the exynos object, so we don't have any issues > Ah, right. However, it does not seem like good way. > with upcasting in exynos dma-buf code. The same applies to i915 > dma-buf code, my follow-up patch just makes the code a bit safer. > > > > > > However, I think not only exynos could go to common drm_gem_dmabuf > directly > > but also it would make your patch set to be complete set if you remove > > exynos related codes from your patch set. Otherwise, we have to work > twice. > > one is the additional work for resolving exynos broken issue by your > patch > > set, and other is to replace existing dmabuf stuff of exynos to common > > drm_gem_dmabuf. > > Yeah np, I'll drop exynos then. > Thanks a lot. :) 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-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
> -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Thursday, August 08, 2013 8:21 AM > To: Inki Dae > Cc: Daniel Vetter; Intel Graphics Development; DRI Development > Subject: Re: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in > i915/exynos drivers > > On Wed, Aug 07, 2013 at 09:37:52PM +0900, Inki Dae wrote: > > 2013/8/7 Daniel Vetter > > > > > On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae wrote: > > > > > > > > > > > > 2013/8/7 Daniel Vetter > > > >> > > > >> On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: > > > >> > On 08/07/2013 06:55 PM, Daniel Vetter wrote: > > > >> > >On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae > > > wrote: > > > >> > >>>-Original Message- > > > >> > >>>From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] > > > >> > >>>Sent: Wednesday, August 07, 2013 6:15 PM > > > >> > >>>To: DRI Development > > > >> > >>>Cc: Intel Graphics Development; Daniel Vetter; Inki Dae > > > >> > >>>Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in > > > >> > >>> i915/exynos > > > >> > >>>drivers > > > >> > >>> > > > >> > >>>Note that this is slightly tricky since both drivers store > their > > > >> > >>>native objects in dma_buf->priv. But both also embed the base > > > >> > >>>drm_gem_object at the first position, so the implicit cast is > ok. > > > >> > >>> > > > >> > >>>To use the release helper we need to export it, too. > > > >> > >>Yeah, may I repost this patch with additional work? We also > need to > > > >> > >> export > > > >> > >>with a gem object instead of specific one like you did. > > > >> > > > > >> > I think dmabuf stuff of exynos can be replaced to common > > > drm_gem_dmabuf. > > > >> > Already dmabuf stuff of drm_gem_cma_helper.c was substituted to > common > > > >> > drm_gem_dmabuf with low-level hook functions to use prime helpers. > > > >> > > > >> Ah, but that can easily be done on top of this, right? > > > > > > > > > > > > Daniel, could you remove exynos related codes from your patch set? > Your > > > > patch set would make exynos broken because you didn't consider > exporting > > > > with a gem object for exynos like [PATCH 3/3] drm/i915: explicit > store > > > base > > > > gem object in dma_buf->priv. So I think your patch set is not > complete > > > set, > > > > and That is why exynos needs the additional work I mentioned above. > So I > > > > just wanted to repost your patch set + new one. > > > > > > Nope, my patch should not break exynos since the base gem_object is > > > the first member of the exynos object, so we don't have any issues > > > > > > > Ah, right. However, it does not seem like good way. > > > > > > > with upcasting in exynos dma-buf code. The same applies to i915 > > > dma-buf code, my follow-up patch just makes the code a bit safer. > > > > > > > > > > > > > > > > > > > > > > However, I think not only exynos could go to common drm_gem_dmabuf > > > directly > > > > but also it would make your patch set to be complete set if you > remove > > > > exynos related codes from your patch set. Otherwise, we have to work > > > twice. > > > > one is the additional work for resolving exynos broken issue by your > > > patch > > > > set, and other is to replace existing dmabuf stuff of exynos to > common > > > > drm_gem_dmabuf. > > > > > > Yeah np, I'll drop exynos then. > > > > > > > Thanks a lot. :) > > Ah, I remember again why I want to also convert over exynos to the common > dma buf release function: Later patches in my prime locking series will > change things in there to avoid a userspace-triggerable oops. If we leave > out exynos it'll break rather badly for dma-buf export. > > I need to think a bit more about what stuff looks like atm, but if I > resend those parts I'll in
[Intel-gfx] [PATCH 1/4] drm/exynos: explicit store base gem object in dma_buf->priv
Signed-off-by: Inki Dae
Signed-off-by: Kyungmin Park
---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index 3cd56e1..fd76449 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -22,6 +22,11 @@ struct exynos_drm_dmabuf_attachment {
bool is_mapped;
};
+static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_exynos_gem_obj(buf->priv);
+}
+
static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
struct device *dev,
struct dma_buf_attachment *attach)
@@ -63,7 +68,7 @@ static struct sg_table *
enum dma_data_direction dir)
{
struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
- struct exynos_drm_gem_obj *gem_obj = attach->dmabuf->priv;
+ struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
struct drm_device *dev = gem_obj->base.dev;
struct exynos_drm_gem_buf *buf;
struct scatterlist *rd, *wr;
@@ -180,7 +185,7 @@ struct dma_buf *exynos_dmabuf_prime_export(struct
drm_device *drm_dev,
{
struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
- return dma_buf_export(exynos_gem_obj, &exynos_dmabuf_ops,
+ return dma_buf_export(obj, &exynos_dmabuf_ops,
exynos_gem_obj->base.size, flags);
}
@@ -198,8 +203,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct
drm_device *drm_dev,
if (dma_buf->ops == &exynos_dmabuf_ops) {
struct drm_gem_object *obj;
- exynos_gem_obj = dma_buf->priv;
- obj = &exynos_gem_obj->base;
+ obj = dma_buf->priv;
/* is it from our device? */
if (obj->dev == drm_dev) {
--
1.7.5.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/exynos: explicit store base gem object in dma_buf->priv
Hi, Daniel. You can repost your patch set including the below my patch. And
my patch numbering is wrong. Sorry about that.
[PATCH 1/4] -> [PATCH 4/4]
Thanks,
Inki Dae
> -Original Message-
> From: Inki Dae [mailto:inki@samsung.com]
> Sent: Thursday, August 08, 2013 1:40 PM
> To: dan...@ffwll.ch
> Cc: dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Inki
> Dae; Kyungmin Park
> Subject: [PATCH 1/4] drm/exynos: explicit store base gem object in
> dma_buf->priv
>
> Signed-off-by: Inki Dae
> Signed-off-by: Kyungmin Park
> ---
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 12
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> index 3cd56e1..fd76449 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> @@ -22,6 +22,11 @@ struct exynos_drm_dmabuf_attachment {
> bool is_mapped;
> };
>
> +static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
> +{
> + return to_exynos_gem_obj(buf->priv);
> +}
> +
> static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
> struct device *dev,
> struct dma_buf_attachment *attach)
> @@ -63,7 +68,7 @@ static struct sg_table *
> enum dma_data_direction dir)
> {
> struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
> - struct exynos_drm_gem_obj *gem_obj = attach->dmabuf->priv;
> + struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
> struct drm_device *dev = gem_obj->base.dev;
> struct exynos_drm_gem_buf *buf;
> struct scatterlist *rd, *wr;
> @@ -180,7 +185,7 @@ struct dma_buf *exynos_dmabuf_prime_export(struct
> drm_device *drm_dev,
> {
> struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>
> - return dma_buf_export(exynos_gem_obj, &exynos_dmabuf_ops,
> + return dma_buf_export(obj, &exynos_dmabuf_ops,
> exynos_gem_obj->base.size, flags);
> }
>
> @@ -198,8 +203,7 @@ struct drm_gem_object
> *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> if (dma_buf->ops == &exynos_dmabuf_ops) {
> struct drm_gem_object *obj;
>
> - exynos_gem_obj = dma_buf->priv;
> - obj = &exynos_gem_obj->base;
> + obj = dma_buf->priv;
>
> /* is it from our device? */
> if (obj->dev == drm_dev) {
> --
> 1.7.5.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
2012/11/2 Imre Deak > The patchset adds the missing event_lock when accessing the > vblank_event_list in drm_vblank_off() and as preparation for this > also fixes a few other issues in the exynos driver. > > This is also a dependency for Rob Clark's drm_send_vblank_event() > rework as that would trigger a warning for the unhold event_lock without > this changeset. > > The exynos changes are only compile tested, the rest is tested on an > Intel IVB machine on top of drm-intel-nightly + drm_send_vblank_event() > rework, with i-g-t/flip_test. > > Hi Imre, Works fine. But we should wait for Rob's patch set to be merged to -next, and this may be rebased on top of latest Rob's patch set again. Thanks, Inki Dae > In v2: > - Instead of fixing the event_lock vs. vblank_time_lock lockdep issue in > drm_handle_vblank(), fix it in the exynos driver. This looks like the > more logical thing to do and this way we also have a smaller impact on > the rest of DRM code. > > Imre Deak (4): > drm/exynos: hold event_lock while accessing pageflip_event_list > drm/exynos: call drm_vblank_put for each queued flip event > drm/exynos: fix lockdep for event_lock wrt. vbl_time_lock > drm: hold event_lock while accessing vblank_event_list > > drivers/gpu/drm/drm_irq.c|3 +++ > drivers/gpu/drm/exynos/exynos_drm_crtc.c |5 + > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 20 +--- > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 20 +--- > drivers/gpu/drm/exynos/exynos_mixer.c| 11 +-- > 5 files changed, 11 insertions(+), 48 deletions(-) > > -- > 1.7.9.5 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
2012/11/7 Imre Deak > On Wed, 2012-11-07 at 18:31 +0900, Inki Dae wrote: > > 2012/11/2 Imre Deak > > The patchset adds the missing event_lock when accessing the > > vblank_event_list in drm_vblank_off() and as preparation for > > this > > also fixes a few other issues in the exynos driver. > > This is also a dependency for Rob Clark's > > drm_send_vblank_event() > > rework as that would trigger a warning for the unhold > > event_lock without > > this changeset. > > The exynos changes are only compile tested, the rest is tested > > on an > > Intel IVB machine on top of drm-intel-nightly + > > drm_send_vblank_event() > > rework, with i-g-t/flip_test. > > Hi Imre, > > Works fine. But we should wait for Rob's patch set to be merged to > > -next, and this may be rebased on top of latest Rob's patch set again. > > Ok, thanks for checking this. I assume then that this patchset will get > merged through your tree. > > I think Rob's patchset depends on this, so ideally this should go first. > Otherwise the i915 driver would trigger the WARN in his patchset due to > the unheld event_lock. > Ok, but I merge it first, shouldn't Rob's patch set be rebased? Anyway this is minor issue so I could resolve it. And it seems like that your patch set has no dependency of Rob's. I mean that your patch set worked fine without Rob's. Thanks, Inki Dae > > --Imre > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
2012/11/8 Rob Clark > On Wed, Nov 7, 2012 at 10:25 AM, Inki Dae wrote: > > > > > > 2012/11/7 Imre Deak > >> > >> On Wed, 2012-11-07 at 18:31 +0900, Inki Dae wrote: > >> > 2012/11/2 Imre Deak > >> > The patchset adds the missing event_lock when accessing the > >> > vblank_event_list in drm_vblank_off() and as preparation for > >> > this > >> > also fixes a few other issues in the exynos driver. > >> > This is also a dependency for Rob Clark's > >> > drm_send_vblank_event() > >> > rework as that would trigger a warning for the unhold > >> > event_lock without > >> > this changeset. > >> > The exynos changes are only compile tested, the rest is tested > >> > on an > >> > Intel IVB machine on top of drm-intel-nightly + > >> > drm_send_vblank_event() > >> > rework, with i-g-t/flip_test. > >> > Hi Imre, > >> > Works fine. But we should wait for Rob's patch set to be merged to > >> > -next, and this may be rebased on top of latest Rob's patch set again. > >> > >> Ok, thanks for checking this. I assume then that this patchset will get > >> merged through your tree. > >> > >> I think Rob's patchset depends on this, so ideally this should go first. > >> Otherwise the i915 driver would trigger the WARN in his patchset due to > >> the unheld event_lock. > > > > > > Ok, but I merge it first, shouldn't Rob's patch set be rebased? Anyway > this > > is minor issue so I could resolve it. And it seems like that your patch > set > > has no dependency of Rob's. I mean that your patch set worked fine > without > > Rob's. > > I think there should be no hard dependency on my patch set.. the only > connection is that my patchset without this patch will start showing > the WARN_ON() traces > > Right, My concern was just merge conflict. > BR, > -R > > > Thanks, > > Inki Dae > > > >> > >> > >> --Imre > >> > >> > >> ___ > >> dri-devel mailing list > >> dri-de...@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/exynos: reorder framebuffer init sequence
> -Original Message-
> From: dri-devel-bounces+inki.dae=samsung@lists.freedesktop.org
> [mailto:dri-devel-bounces+inki.dae=samsung@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
> ---
> 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-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/exynos: reorder framebuffer init sequence
2012/12/13 Daniel Vetter > 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 wrote: > >> -Original Message- > >> From: dri-devel-bounces+inki.dae=samsung@lists.freedesktop.org > >> [mailto:dri-devel-bounces+inki.dae=samsung@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-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/exynos: reorder framebuffer init sequence
2012/12/14 Daniel Vetter > On Thu, Dec 13, 2012 at 4:16 PM, Inki Dae 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-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/11] drm: add plane support
Hi, all.
SW Kim gmail.com> writes:
>
> Daniel Vetter ffwll.ch> writes:
>
> [snip]
>
> > > struct drm_mode_fb_cmd2 {
> > > __u32 fb_id;
> > > __u32 width, height;
> > > __u32 pixel_format; /* fourcc code from videodev2.h */
> > > __u64 priv; /* private data, depends on pixelformat */
> > >
>
> [snip]
>
> > > struct {
> > > __u32 pitch;
> > > /* driver specific handle */
> > > __u32 handle;
> >
> > Why not just add a
> > __u32 offset;
> > __u32 rsvd;
>
> For normal NV12 format, I agree that just offset is enough.
> But considering more specific format like NV12M, that has two separated
memory
> spaces, IMHO, it needs handle per each plane.
>
> Each plane of NV12M has it own base address and space between these two
memory
> spaces can be used by others.
>
> You can refer following link about NV12M used by V4L2.
> http://linuxtv.org/downloads/v4l-dvb-apis/V4L2-PIX-FMT-NV12M.html
>
> > and call it a day. This thing is pretty big already, so that bloat doesn't
> > matter that much. Maybe spec that buffer[0].offset must be zero. With that
> > you can also do I420 with the following layout
> > +---+
> > |YYY|
> > |YYY|
> > |YYY|
> > |YYY|
> > +---+---+
> > |UUU|VVV|
> > |UUU|VVV|
> > +---+---+
> >
> > i.e. stride_U == stride_V, with their lines meshed into one line.
> > -Daniel
> >
> > > } buffer[16];
>
> and just question, is there any meaning about buffer array size 16?
>
> > > };
>
> [snip]
>
> Thanks and Regards,
> - SW Kim
>
I have a opinion for multi planer. before that, I'd like to mention some
considerations.
I think adding any variables such as offset or handle(as plane count) to
drm_mode_fb_cmd2 structure could occurs that in application point of view,
user could be confused because with the ways above, user needs to allocate
buffers as plane count(it depends on pixel format) and also user should know
buffer size according to pixel format. so I think it's good way that user sets
only pixel format and resolution(width, height) and then specific gem
framework allocates buffers according to pixel format as plane count and then
user sets the gem handle to drm_mode_fb_cmd2 structure. specific gem framework
could get all buffers through only the gem handle.
below is my simple idea.
1. user requests buffer allocation with pixel format and resolution through
gem framework.
2. gem framework checks pixel format.
3. specific gem framework allocates buffers as plane count according to the
pixel format. (please, know that gem framework provides just interface so
acctual implementation would be done at specific gem framework)
4. user gets the gem handle from gem framework.
5. user sets the gem handle to specific drm framebuffer through
drm_mode_addfb2 function.
6. user requests setcrtc with fb id and crtc id.
7. drm framework sets framebuffer(corresponding to fb id) to drm_mode_set.
8. crtc calls set_config callbacks to configure hardware.
9. specific crtc framework gets all buffers through framebuffer(actually, it
would be specific framebuffer)and sets them to overlay registers of hardware
appropriately.
like this, how about using framebuffer and gem framework instead of plane? I'd
be glad to give me your opinions.
Thank you,
Inki Dae.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/11] drm: add plane support
Hi, Jesse. Jesse Barnes virtuousgeek.org> writes: > > On Mon, 31 Oct 2011 11:40:57 + (UTC) > Inki Dae gmail.com> wrote: > > below is my simple idea. > > 1. user requests buffer allocation with pixel format and resolution through > > gem framework. > > 2. gem framework checks pixel format. > > 3. specific gem framework allocates buffers as plane count according to the > > pixel format. (please, know that gem framework provides just interface so > > acctual implementation would be done at specific gem framework) > > 4. user gets the gem handle from gem framework. > > 5. user sets the gem handle to specific drm framebuffer through > > drm_mode_addfb2 function. > > 6. user requests setcrtc with fb id and crtc id. > > 7. drm framework sets framebuffer(corresponding to fb id) to drm_mode_set. > > 8. crtc calls set_config callbacks to configure hardware. > > 9. specific crtc framework gets all buffers through framebuffer(actually, it > > would be specific framebuffer)and sets them to overlay registers of hardware > > appropriately. > > > > > > like this, how about using framebuffer and gem framework instead of plane? I'd > > be glad to give me your opinions. > > I'm not opposed to pushing the multi-object stuff into GEM instead, but > I don't think step (9) will be enough to avoid having a separate plane > object. Say the user passes in 3 RGB buffers. How do you know which > one is the primary and which are overlays? Or are you saying the fb > would have that information as well? If so, it would be a little > harder to specify things like blending options or colorspace correction > on a per-plane basis... > > But maybe I'm missing some part of things; do you have any patches to > illustrate what you mean? I can see how it might work, but I don't > know if it would be any simpler or cleaner than exposing plane objects. > > Thanks, Sorry, there is my missing point. please, ignor step 6 ~ 9. if user requests setplane then drm_mode_setplane function gets fb and crtc object to update the overlay corresponding to plane id. at that time I think we could know which overlay should be set and how many buffers does the overlay has through specific framebuffer object if specific framebuffer has gem object. in our case, specific framebuffer has gem object. and I would try to implement multi planer the way I mentioned if need and I will post the patch. this work takes about a week. I think the way that user allocates buffers for multi planer only with pixel format and resolution(width, height) would be good. but there could be more good ways then this way. please give me your opinion and advices. Thank you, Inki Dae. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/11] drm/i915: add SNB video sprite support
Hi, Jesse. drm_plane structure has format_types and format_conunt that they was set at booting time. but they aren't used anywhere. at intel_update_plane(), I guess they could be used to check if the format type(fb->pixel_format) from setplane () is supported or not, comparing to plane->format_types[n]; otherwise, could you please tell me about the purpose that format_types[] and format_count are used? Thank you, Inki Dae. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
