All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark yao <mark.yao@rock-chips.com>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>, linux-kernel@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event v2
Date: Fri, 01 Apr 2016 19:54:47 +0800	[thread overview]
Message-ID: <56FE6187.1010004@rock-chips.com> (raw)
In-Reply-To: <56FE5FD2.2050106@collabora.com>

On 2016年04月01日 19:47, Tomeu Vizoso wrote:
> On 04/01/2016 01:26 PM, Mark yao wrote:
>> On 2016年03月31日 16:08, Tomeu Vizoso wrote:
>>> As per the docs, atomic_commit should return -EBUSY "if an asycnhronous
>>> updated is requested and there is an earlier updated pending".
>>>
>>> v2: Use the status of the workqueue instead of vop->event, and don't add
>>> a superfluous wait on the workqueue.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> index 3b8f652698f8..285f8cd5afe1 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> @@ -282,6 +282,9 @@ int rockchip_drm_atomic_commit(struct drm_device *dev,
>>>   	struct rockchip_atomic_commit *commit = &private->commit;
>>>   	int ret;
>>>   
>>> +	if (async && work_busy(&commit->work))
>>> +		return -EBUSY;
>>> +
>> Sorry for reply late.
>>
>> There is a comment on work_busy function describe :
>>
>>      "the test result is  unreliable and only useful as advisory hints or
>> for debugging."
>>
>> I don't know if it's suitable to use it here, does some guys know it?
> I'm not sure, but if the reason is the caveat explained in
> find_worker_executing_work(), then it's probably safe (and would explain
> how the function is used in other parts in the kernel).
>
>> And then, the "flush_work(&commit->work);" is no needed if return -EBUSY
>> here.
>> you can remove it at this patch.
> We still need to wait if it's being called in sync mode.
>
> Regards,
>
> Tomeu
Hi TomeuHi

on sync mode, flush is no needed, because that:
1, there is mutex_lock/mutex_unlock on this context, So only single 
process run into commit work;

2, sync mode will block on:
rockchip_atomic_commit_complete-->rockchip_atomic_wait_for_complete,

Thanks.

>
>>>   	ret = drm_atomic_helper_prepare_planes(dev, state);
>>>   	if (ret)
>>>   		return ret;
>>
>>
>> -- 
>> Mark Yao
>>
>
>
>


-- 
Mark Yao


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: mark.yao@rock-chips.com (Mark yao)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event v2
Date: Fri, 01 Apr 2016 19:54:47 +0800	[thread overview]
Message-ID: <56FE6187.1010004@rock-chips.com> (raw)
In-Reply-To: <56FE5FD2.2050106@collabora.com>

On 2016?04?01? 19:47, Tomeu Vizoso wrote:
> On 04/01/2016 01:26 PM, Mark yao wrote:
>> On 2016?03?31? 16:08, Tomeu Vizoso wrote:
>>> As per the docs, atomic_commit should return -EBUSY "if an asycnhronous
>>> updated is requested and there is an earlier updated pending".
>>>
>>> v2: Use the status of the workqueue instead of vop->event, and don't add
>>> a superfluous wait on the workqueue.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> index 3b8f652698f8..285f8cd5afe1 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> @@ -282,6 +282,9 @@ int rockchip_drm_atomic_commit(struct drm_device *dev,
>>>   	struct rockchip_atomic_commit *commit = &private->commit;
>>>   	int ret;
>>>   
>>> +	if (async && work_busy(&commit->work))
>>> +		return -EBUSY;
>>> +
>> Sorry for reply late.
>>
>> There is a comment on work_busy function describe :
>>
>>      "the test result is  unreliable and only useful as advisory hints or
>> for debugging."
>>
>> I don't know if it's suitable to use it here, does some guys know it?
> I'm not sure, but if the reason is the caveat explained in
> find_worker_executing_work(), then it's probably safe (and would explain
> how the function is used in other parts in the kernel).
>
>> And then, the "flush_work(&commit->work);" is no needed if return -EBUSY
>> here.
>> you can remove it at this patch.
> We still need to wait if it's being called in sync mode.
>
> Regards,
>
> Tomeu
Hi Tomeu??

on sync mode, flush is no needed, because that:
1, there is mutex_lock/mutex_unlock on this context, So only single 
process run into commit work;

2, sync mode will block on:
rockchip_atomic_commit_complete-->rockchip_atomic_wait_for_complete,

Thanks.

>
>>>   	ret = drm_atomic_helper_prepare_planes(dev, state);
>>>   	if (ret)
>>>   		return ret;
>>
>>
>> -- 
>> ?ark Yao
>>
>
>
>


-- 
?ark Yao

WARNING: multiple messages have this Message-ID (diff)
From: Mark yao <mark.yao@rock-chips.com>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>, linux-kernel@vger.kernel.org
Cc: David Airlie <airlied@linux.ie>, Heiko Stuebner <heiko@sntech.de>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Subject: Re: [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event v2
Date: Fri, 01 Apr 2016 19:54:47 +0800	[thread overview]
Message-ID: <56FE6187.1010004@rock-chips.com> (raw)
In-Reply-To: <56FE5FD2.2050106@collabora.com>

On 2016年04月01日 19:47, Tomeu Vizoso wrote:
> On 04/01/2016 01:26 PM, Mark yao wrote:
>> On 2016年03月31日 16:08, Tomeu Vizoso wrote:
>>> As per the docs, atomic_commit should return -EBUSY "if an asycnhronous
>>> updated is requested and there is an earlier updated pending".
>>>
>>> v2: Use the status of the workqueue instead of vop->event, and don't add
>>> a superfluous wait on the workqueue.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> index 3b8f652698f8..285f8cd5afe1 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>>> @@ -282,6 +282,9 @@ int rockchip_drm_atomic_commit(struct drm_device *dev,
>>>   	struct rockchip_atomic_commit *commit = &private->commit;
>>>   	int ret;
>>>   
>>> +	if (async && work_busy(&commit->work))
>>> +		return -EBUSY;
>>> +
>> Sorry for reply late.
>>
>> There is a comment on work_busy function describe :
>>
>>      "the test result is  unreliable and only useful as advisory hints or
>> for debugging."
>>
>> I don't know if it's suitable to use it here, does some guys know it?
> I'm not sure, but if the reason is the caveat explained in
> find_worker_executing_work(), then it's probably safe (and would explain
> how the function is used in other parts in the kernel).
>
>> And then, the "flush_work(&commit->work);" is no needed if return -EBUSY
>> here.
>> you can remove it at this patch.
> We still need to wait if it's being called in sync mode.
>
> Regards,
>
> Tomeu
Hi TomeuHi

on sync mode, flush is no needed, because that:
1, there is mutex_lock/mutex_unlock on this context, So only single 
process run into commit work;

2, sync mode will block on:
rockchip_atomic_commit_complete-->rockchip_atomic_wait_for_complete,

Thanks.

>
>>>   	ret = drm_atomic_helper_prepare_planes(dev, state);
>>>   	if (ret)
>>>   		return ret;
>>
>>
>> -- 
>> Mark Yao
>>
>
>
>


-- 
Mark Yao

  reply	other threads:[~2016-04-01 11:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-31  8:08 [PATCH] drm/rockchip: Return -EBUSY if there's already a pending flip event v2 Tomeu Vizoso
2016-03-31  8:08 ` Tomeu Vizoso
2016-03-31  8:08 ` Tomeu Vizoso
2016-04-01 11:26 ` Mark yao
2016-04-01 11:47   ` Tomeu Vizoso
2016-04-01 11:47     ` Tomeu Vizoso
2016-04-01 11:47     ` Tomeu Vizoso
2016-04-01 11:54     ` Mark yao [this message]
2016-04-01 11:54       ` Mark yao
2016-04-01 11:54       ` Mark yao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56FE6187.1010004@rock-chips.com \
    --to=mark.yao@rock-chips.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=tomeu.vizoso@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.