* [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
@ 2016-06-15 10:08 Daniel Vetter
2016-06-15 10:21 ` Liviu.Dudau
2016-06-16 15:30 ` Oded Gabbay
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-06-15 10:08 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Liviu.Dudau, Daniel Vetter
It's not obvious at first sight that this is a fastpath, make that
clearer with a goto. Fallout from a discussion with Liviu on irc.
Cc: Liviu.Dudau@arm.com
Acked-by: Liviu.Dudau@arm.com
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ---
drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
include/drm/drm_crtc.h | 23 ++++------------------
4 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
index ec4036a09f3e..a625b9137da2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
@@ -187,12 +187,12 @@ int init_pipelines(struct device_queue_manager *dqm,
unsigned int get_first_pipe(struct device_queue_manager *dqm);
unsigned int get_pipes_num(struct device_queue_manager *dqm);
-extern inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
+static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
{
return (pdd->lds_base >> 16) & 0xFF;
}
-extern inline unsigned int
+static inline unsigned int
get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd)
{
return (pdd->lds_base >> 60) & 0x0E;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index d0d5f4baf72d..80113c335966 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -617,10 +617,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd);
int kfd_init_apertures(struct kfd_process *process);
/* Queue Context Management */
-inline uint32_t lower_32(uint64_t x);
-inline uint32_t upper_32(uint64_t x);
struct cik_sdma_rlc_registers *get_sdma_mqd(void *mqd);
-inline uint32_t get_sdma_base_addr(struct cik_sdma_rlc_registers *m);
int init_queue(struct queue **q, struct queue_properties properties);
void uninit_queue(struct queue *q);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 716aa535eb98..0556c95b7ddb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
/* commit_list borrows our reference, need to remove before we
* clean up our drm_atomic_state. But only after it actually
* completed, otherwise subsequent commits won't stall properly. */
- if (try_wait_for_completion(&commit->flip_done)) {
- list_del(&commit->commit_entry);
- spin_unlock(&crtc->commit_lock);
- continue;
- }
+ if (try_wait_for_completion(&commit->flip_done))
+ goto del_commit:
spin_unlock(&crtc->commit_lock);
@@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
crtc->base.id, crtc->name);
spin_lock(&crtc->commit_lock);
+del_commit:
list_del(&commit->commit_entry);
spin_unlock(&crtc->commit_lock);
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3c84ddc7e4c8..a39e1f17a20e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2820,29 +2820,14 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
#define drm_for_each_crtc(crtc, dev) \
list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
-static inline void
-assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
-{
- /*
- * The connector hotadd/remove code currently grabs both locks when
- * updating lists. Hence readers need only hold either of them to be
- * safe and the check amounts to
- *
- * WARN_ON(not_holding(A) && not_holding(B)).
- */
- WARN_ON(!mutex_is_locked(&mode_config->mutex) &&
- !drm_modeset_is_locked(&mode_config->connection_mutex));
-}
-
#define drm_for_each_connector(connector, dev) \
/* loop to wrap everything into a srcu read-side critical section */ \
- for (bool __conn_loop_srcu = true, \
- int __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
- __conn_loop_srcu; __conn_loop_srcu = false, \
+ for (int __conn_loop_srcu = 1, \
+ __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
+ __conn_loop_srcu; __conn_loop_srcu = 0, \
srcu_read_unlock(&drm_connector_list_srcu, __conn_loop_srcu_ret)) \
/* loop to iterate over the connector_list, rcu-protected */ \
- for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
- connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
+ for (connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
struct drm_connector, head); \
&connector->head != (&(dev)->mode_config.connector_list); \
connector = list_entry_rcu(connector->head.next, \
--
2.8.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
2016-06-15 10:08 [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit Daniel Vetter
@ 2016-06-15 10:21 ` Liviu.Dudau
2016-06-16 15:30 ` Oded Gabbay
1 sibling, 0 replies; 7+ messages in thread
From: Liviu.Dudau @ 2016-06-15 10:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, DRI Development
On Wed, Jun 15, 2016 at 12:08:29PM +0200, Daniel Vetter wrote:
> It's not obvious at first sight that this is a fastpath, make that
> clearer with a goto. Fallout from a discussion with Liviu on irc.
>
> Cc: Liviu.Dudau@arm.com
> Acked-by: Liviu.Dudau@arm.com
Nope, I did not agree to *all* those changes, only the drm_atomic_helper.c one :)
Best regards,
Liviu
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++--
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ---
> drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> include/drm/drm_crtc.h | 23 ++++------------------
> 4 files changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> index ec4036a09f3e..a625b9137da2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> @@ -187,12 +187,12 @@ int init_pipelines(struct device_queue_manager *dqm,
> unsigned int get_first_pipe(struct device_queue_manager *dqm);
> unsigned int get_pipes_num(struct device_queue_manager *dqm);
>
> -extern inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
> +static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
> {
> return (pdd->lds_base >> 16) & 0xFF;
> }
>
> -extern inline unsigned int
> +static inline unsigned int
> get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd)
> {
> return (pdd->lds_base >> 60) & 0x0E;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index d0d5f4baf72d..80113c335966 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -617,10 +617,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd);
> int kfd_init_apertures(struct kfd_process *process);
>
> /* Queue Context Management */
> -inline uint32_t lower_32(uint64_t x);
> -inline uint32_t upper_32(uint64_t x);
> struct cik_sdma_rlc_registers *get_sdma_mqd(void *mqd);
> -inline uint32_t get_sdma_base_addr(struct cik_sdma_rlc_registers *m);
>
> int init_queue(struct queue **q, struct queue_properties properties);
> void uninit_queue(struct queue *q);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 716aa535eb98..0556c95b7ddb 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> /* commit_list borrows our reference, need to remove before we
> * clean up our drm_atomic_state. But only after it actually
> * completed, otherwise subsequent commits won't stall properly. */
> - if (try_wait_for_completion(&commit->flip_done)) {
> - list_del(&commit->commit_entry);
> - spin_unlock(&crtc->commit_lock);
> - continue;
> - }
> + if (try_wait_for_completion(&commit->flip_done))
> + goto del_commit:
>
> spin_unlock(&crtc->commit_lock);
>
> @@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> crtc->base.id, crtc->name);
>
> spin_lock(&crtc->commit_lock);
> +del_commit:
> list_del(&commit->commit_entry);
> spin_unlock(&crtc->commit_lock);
> }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3c84ddc7e4c8..a39e1f17a20e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2820,29 +2820,14 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
> #define drm_for_each_crtc(crtc, dev) \
> list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>
> -static inline void
> -assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
> -{
> - /*
> - * The connector hotadd/remove code currently grabs both locks when
> - * updating lists. Hence readers need only hold either of them to be
> - * safe and the check amounts to
> - *
> - * WARN_ON(not_holding(A) && not_holding(B)).
> - */
> - WARN_ON(!mutex_is_locked(&mode_config->mutex) &&
> - !drm_modeset_is_locked(&mode_config->connection_mutex));
> -}
> -
> #define drm_for_each_connector(connector, dev) \
> /* loop to wrap everything into a srcu read-side critical section */ \
> - for (bool __conn_loop_srcu = true, \
> - int __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
> - __conn_loop_srcu; __conn_loop_srcu = false, \
> + for (int __conn_loop_srcu = 1, \
> + __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
> + __conn_loop_srcu; __conn_loop_srcu = 0, \
> srcu_read_unlock(&drm_connector_list_srcu, __conn_loop_srcu_ret)) \
> /* loop to iterate over the connector_list, rcu-protected */ \
> - for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
> - connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
> + for (connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
> struct drm_connector, head); \
> &connector->head != (&(dev)->mode_config.connector_list); \
> connector = list_entry_rcu(connector->head.next, \
> --
> 2.8.1
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
2016-06-15 10:08 [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit Daniel Vetter
2016-06-15 10:21 ` Liviu.Dudau
@ 2016-06-16 15:30 ` Oded Gabbay
2016-06-16 16:11 ` Daniel Vetter
1 sibling, 1 reply; 7+ messages in thread
From: Oded Gabbay @ 2016-06-16 15:30 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Liviu.Dudau, DRI Development
On Wed, Jun 15, 2016 at 1:08 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> It's not obvious at first sight that this is a fastpath, make that
> clearer with a goto. Fallout from a discussion with Liviu on irc.
>
> Cc: Liviu.Dudau@arm.com
> Acked-by: Liviu.Dudau@arm.com
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++--
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ---
Hi Daniel,
I'm a bit confused, how the kfd changes relate to this specific commit ?
The commit message seems to talk about something else.
Oded
> drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> include/drm/drm_crtc.h | 23 ++++------------------
> 4 files changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> index ec4036a09f3e..a625b9137da2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> @@ -187,12 +187,12 @@ int init_pipelines(struct device_queue_manager *dqm,
> unsigned int get_first_pipe(struct device_queue_manager *dqm);
> unsigned int get_pipes_num(struct device_queue_manager *dqm);
>
> -extern inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
> +static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd)
> {
> return (pdd->lds_base >> 16) & 0xFF;
> }
>
> -extern inline unsigned int
> +static inline unsigned int
> get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd)
> {
> return (pdd->lds_base >> 60) & 0x0E;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index d0d5f4baf72d..80113c335966 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -617,10 +617,7 @@ int kgd2kfd_resume(struct kfd_dev *kfd);
> int kfd_init_apertures(struct kfd_process *process);
>
> /* Queue Context Management */
> -inline uint32_t lower_32(uint64_t x);
> -inline uint32_t upper_32(uint64_t x);
> struct cik_sdma_rlc_registers *get_sdma_mqd(void *mqd);
> -inline uint32_t get_sdma_base_addr(struct cik_sdma_rlc_registers *m);
>
> int init_queue(struct queue **q, struct queue_properties properties);
> void uninit_queue(struct queue *q);
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 716aa535eb98..0556c95b7ddb 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> /* commit_list borrows our reference, need to remove before we
> * clean up our drm_atomic_state. But only after it actually
> * completed, otherwise subsequent commits won't stall properly. */
> - if (try_wait_for_completion(&commit->flip_done)) {
> - list_del(&commit->commit_entry);
> - spin_unlock(&crtc->commit_lock);
> - continue;
> - }
> + if (try_wait_for_completion(&commit->flip_done))
> + goto del_commit:
>
> spin_unlock(&crtc->commit_lock);
>
> @@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> crtc->base.id, crtc->name);
>
> spin_lock(&crtc->commit_lock);
> +del_commit:
> list_del(&commit->commit_entry);
> spin_unlock(&crtc->commit_lock);
> }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3c84ddc7e4c8..a39e1f17a20e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2820,29 +2820,14 @@ static inline void drm_connector_unreference(struct drm_connector *connector)
> #define drm_for_each_crtc(crtc, dev) \
> list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>
> -static inline void
> -assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
> -{
> - /*
> - * The connector hotadd/remove code currently grabs both locks when
> - * updating lists. Hence readers need only hold either of them to be
> - * safe and the check amounts to
> - *
> - * WARN_ON(not_holding(A) && not_holding(B)).
> - */
> - WARN_ON(!mutex_is_locked(&mode_config->mutex) &&
> - !drm_modeset_is_locked(&mode_config->connection_mutex));
> -}
> -
> #define drm_for_each_connector(connector, dev) \
> /* loop to wrap everything into a srcu read-side critical section */ \
> - for (bool __conn_loop_srcu = true, \
> - int __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
> - __conn_loop_srcu; __conn_loop_srcu = false, \
> + for (int __conn_loop_srcu = 1, \
> + __conn_loop_srcu_ret = srcu_read_lock(&drm_connector_list_srcu);\
> + __conn_loop_srcu; __conn_loop_srcu = 0, \
> srcu_read_unlock(&drm_connector_list_srcu, __conn_loop_srcu_ret)) \
> /* loop to iterate over the connector_list, rcu-protected */ \
> - for (assert_drm_connector_list_read_locked(&(dev)->mode_config), \
> - connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
> + for (connector = list_entry_rcu((dev)->mode_config.connector_list.next, \
> struct drm_connector, head); \
> &connector->head != (&(dev)->mode_config.connector_list); \
> connector = list_entry_rcu(connector->head.next, \
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
2016-06-16 15:30 ` Oded Gabbay
@ 2016-06-16 16:11 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-06-16 16:11 UTC (permalink / raw)
To: Oded Gabbay; +Cc: Daniel Vetter, Liviu Dudau, DRI Development
On Thu, Jun 16, 2016 at 5:30 PM, Oded Gabbay <oded.gabbay@gmail.com> wrote:
> On Wed, Jun 15, 2016 at 1:08 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> It's not obvious at first sight that this is a fastpath, make that
>> clearer with a goto. Fallout from a discussion with Liviu on irc.
>>
>> Cc: Liviu.Dudau@arm.com
>> Acked-by: Liviu.Dudau@arm.com
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h | 4 ++--
>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 ---
>
> Hi Daniel,
>
> I'm a bit confused, how the kfd changes relate to this specific commit ?
> The commit message seems to talk about something else.
They don't, this is just git fail on my side.
-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
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
2016-06-15 14:29 ` Liviu.Dudau
@ 2016-06-15 15:26 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-06-15 15:26 UTC (permalink / raw)
To: Liviu.Dudau; +Cc: Daniel Vetter, DRI Development, Daniel Vetter
On Wed, Jun 15, 2016 at 03:29:16PM +0100, Liviu.Dudau@arm.com wrote:
> On Wed, Jun 15, 2016 at 12:24:26PM +0200, Daniel Vetter wrote:
> > It's not obvious at first sight that this is a fastpath, make that
> > clearer with a goto. Fallout from a discussion with Liviu on irc.
> >
> > v2: Drop bogus hunks that crept in.
> >
> > Cc: Liviu.Dudau@arm.com
> > Acked-by: Liviu.Dudau@arm.com
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 716aa535eb98..0556c95b7ddb 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> > /* commit_list borrows our reference, need to remove before we
> > * clean up our drm_atomic_state. But only after it actually
> > * completed, otherwise subsequent commits won't stall properly. */
> > - if (try_wait_for_completion(&commit->flip_done)) {
> > - list_del(&commit->commit_entry);
> > - spin_unlock(&crtc->commit_lock);
> > - continue;
> > - }
> > + if (try_wait_for_completion(&commit->flip_done))
> > + goto del_commit:
>
> s/:/;/
Yeah I'm pretty good with posting a rebase mess with this one.
Fixed&applied, thanks.
-Daniel
>
> Liviu
>
> >
> > spin_unlock(&crtc->commit_lock);
> >
> > @@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> > crtc->base.id, crtc->name);
> >
> > spin_lock(&crtc->commit_lock);
> > +del_commit:
> > list_del(&commit->commit_entry);
> > spin_unlock(&crtc->commit_lock);
> > }
> > --
> > 2.8.1
> >
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
2016-06-15 10:24 Daniel Vetter
@ 2016-06-15 14:29 ` Liviu.Dudau
2016-06-15 15:26 ` Daniel Vetter
0 siblings, 1 reply; 7+ messages in thread
From: Liviu.Dudau @ 2016-06-15 14:29 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, DRI Development
On Wed, Jun 15, 2016 at 12:24:26PM +0200, Daniel Vetter wrote:
> It's not obvious at first sight that this is a fastpath, make that
> clearer with a goto. Fallout from a discussion with Liviu on irc.
>
> v2: Drop bogus hunks that crept in.
>
> Cc: Liviu.Dudau@arm.com
> Acked-by: Liviu.Dudau@arm.com
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 716aa535eb98..0556c95b7ddb 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> /* commit_list borrows our reference, need to remove before we
> * clean up our drm_atomic_state. But only after it actually
> * completed, otherwise subsequent commits won't stall properly. */
> - if (try_wait_for_completion(&commit->flip_done)) {
> - list_del(&commit->commit_entry);
> - spin_unlock(&crtc->commit_lock);
> - continue;
> - }
> + if (try_wait_for_completion(&commit->flip_done))
> + goto del_commit:
s/:/;/
Liviu
>
> spin_unlock(&crtc->commit_lock);
>
> @@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
> crtc->base.id, crtc->name);
>
> spin_lock(&crtc->commit_lock);
> +del_commit:
> list_del(&commit->commit_entry);
> spin_unlock(&crtc->commit_lock);
> }
> --
> 2.8.1
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit
@ 2016-06-15 10:24 Daniel Vetter
2016-06-15 14:29 ` Liviu.Dudau
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2016-06-15 10:24 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Liviu.Dudau, Daniel Vetter
It's not obvious at first sight that this is a fastpath, make that
clearer with a goto. Fallout from a discussion with Liviu on irc.
v2: Drop bogus hunks that crept in.
Cc: Liviu.Dudau@arm.com
Acked-by: Liviu.Dudau@arm.com
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 716aa535eb98..0556c95b7ddb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1579,11 +1579,8 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
/* commit_list borrows our reference, need to remove before we
* clean up our drm_atomic_state. But only after it actually
* completed, otherwise subsequent commits won't stall properly. */
- if (try_wait_for_completion(&commit->flip_done)) {
- list_del(&commit->commit_entry);
- spin_unlock(&crtc->commit_lock);
- continue;
- }
+ if (try_wait_for_completion(&commit->flip_done))
+ goto del_commit:
spin_unlock(&crtc->commit_lock);
@@ -1597,6 +1594,7 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state)
crtc->base.id, crtc->name);
spin_lock(&crtc->commit_lock);
+del_commit:
list_del(&commit->commit_entry);
spin_unlock(&crtc->commit_lock);
}
--
2.8.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-16 16:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15 10:08 [PATCH] drm/atomic-helpers: Clear up cleanup_done a bit Daniel Vetter
2016-06-15 10:21 ` Liviu.Dudau
2016-06-16 15:30 ` Oded Gabbay
2016-06-16 16:11 ` Daniel Vetter
2016-06-15 10:24 Daniel Vetter
2016-06-15 14:29 ` Liviu.Dudau
2016-06-15 15:26 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.