diff mbox

[3/5,v2] drm/exynos: allow mulitple layer updates per vsync for mixer

Message ID 1403501545-16482-4-git-send-email-rahul.sharma@samsung.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Rahul Sharma June 23, 2014, 5:32 a.m. UTC
Allowing only one layer update per vsync can cause issues
while there are update available for both layers. There is
a good amount of possibility to loose updates if we allow
single update per vsync.

Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Inki Dae June 24, 2014, 5:21 a.m. UTC | #1
On 2014? 06? 23? 14:32, Rahul Sharma wrote:
> Allowing only one layer update per vsync can cause issues
> while there are update available for both layers. There is
> a good amount of possibility to loose updates if we allow
> single update per vsync.
> 
> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index d359501..6773b03 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>  static void mixer_layer_update(struct mixer_context *ctx)
>  {
>  	struct mixer_resources *res = &ctx->mixer_res;
> -	u32 val;
> -
> -	val = mixer_reg_read(res, MXR_CFG);
>  
> -	/* allow one update per vsync only */
> -	if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
> -		mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
> +	mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);

Rahul, it looks good to me and ok as is. But above codes don't consider
Exynos4 series. In case of Exynos4xxx SoC,
MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
register are reserved fields. So can you work that patch to be
considered for Exynos4xxx SoC? That patch would be additional one.

Anyway, will apply it as is, and I will wait for the additional patch.

Thanks,
Inki Dae

>  }
>  
>  static void mixer_graph_buffer(struct mixer_context *ctx, int win)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Färber June 24, 2014, 11:38 a.m. UTC | #2
Am 24.06.2014 07:21, schrieb Inki Dae:
> On 2014? 06? 23? 14:32, Rahul Sharma wrote:
>> Allowing only one layer update per vsync can cause issues
>> while there are update available for both layers. There is
>> a good amount of possibility to loose updates if we allow
>> single update per vsync.
>>
>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index d359501..6773b03 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>>  static void mixer_layer_update(struct mixer_context *ctx)
>>  {
>>  	struct mixer_resources *res = &ctx->mixer_res;
>> -	u32 val;
>> -
>> -	val = mixer_reg_read(res, MXR_CFG);
>>  
>> -	/* allow one update per vsync only */
>> -	if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
>> -		mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>> +	mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
> 
> Rahul, it looks good to me and ok as is. But above codes don't consider
> Exynos4 series. In case of Exynos4xxx SoC,
> MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
> register are reserved fields. So can you work that patch to be
> considered for Exynos4xxx SoC? That patch would be additional one.
> 
> Anyway, will apply it as is, and I will wait for the additional patch.

If it's not too late, could you fix up "multiple" in the subject? :)

Cheers,
Andreas
Inki Dae June 24, 2014, 2:50 p.m. UTC | #3
2014-06-24 20:38 GMT+09:00 Andreas Färber <afaerber@suse.de>:
> Am 24.06.2014 07:21, schrieb Inki Dae:
>> On 2014? 06? 23? 14:32, Rahul Sharma wrote:
>>> Allowing only one layer update per vsync can cause issues
>>> while there are update available for both layers. There is
>>> a good amount of possibility to loose updates if we allow
>>> single update per vsync.
>>>
>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index d359501..6773b03 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>>>  static void mixer_layer_update(struct mixer_context *ctx)
>>>  {
>>>      struct mixer_resources *res = &ctx->mixer_res;
>>> -    u32 val;
>>> -
>>> -    val = mixer_reg_read(res, MXR_CFG);
>>>
>>> -    /* allow one update per vsync only */
>>> -    if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
>>> -            mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>> +    mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>
>> Rahul, it looks good to me and ok as is. But above codes don't consider
>> Exynos4 series. In case of Exynos4xxx SoC,
>> MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
>> register are reserved fields. So can you work that patch to be
>> considered for Exynos4xxx SoC? That patch would be additional one.
>>
>> Anyway, will apply it as is, and I will wait for the additional patch.
>
> If it's not too late, could you fix up "multiple" in the subject? :)

Corrected. :)

Thanks,
Inki Dae

>
> Cheers,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rahul Sharma June 25, 2014, 10:42 a.m. UTC | #4
Thanks Inki,

One more thing. mixer_layer_update is only called on for mixer version;
MXR_VER_16_0_33_0, MXR_VER_128_0_0_184. This condition
should have taken care of Exynos4 scenarios. What you say?

Regards,
Rahul Sharma.

On 24 June 2014 20:20, Inki Dae <inki.dae@samsung.com> wrote:
> 2014-06-24 20:38 GMT+09:00 Andreas Färber <afaerber@suse.de>:
>> Am 24.06.2014 07:21, schrieb Inki Dae:
>>> On 2014? 06? 23? 14:32, Rahul Sharma wrote:
>>>> Allowing only one layer update per vsync can cause issues
>>>> while there are update available for both layers. There is
>>>> a good amount of possibility to loose updates if we allow
>>>> single update per vsync.
>>>>
>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
>>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> index d359501..6773b03 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>>>>  static void mixer_layer_update(struct mixer_context *ctx)
>>>>  {
>>>>      struct mixer_resources *res = &ctx->mixer_res;
>>>> -    u32 val;
>>>> -
>>>> -    val = mixer_reg_read(res, MXR_CFG);
>>>>
>>>> -    /* allow one update per vsync only */
>>>> -    if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
>>>> -            mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>> +    mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>
>>> Rahul, it looks good to me and ok as is. But above codes don't consider
>>> Exynos4 series. In case of Exynos4xxx SoC,
>>> MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
>>> register are reserved fields. So can you work that patch to be
>>> considered for Exynos4xxx SoC? That patch would be additional one.
>>>
>>> Anyway, will apply it as is, and I will wait for the additional patch.
>>
>> If it's not too late, could you fix up "multiple" in the subject? :)
>
> Corrected. :)
>
> Thanks,
> Inki Dae
>
>>
>> Cheers,
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Inki Dae June 25, 2014, 11:57 a.m. UTC | #5
On 2014? 06? 25? 19:42, Rahul Sharma wrote:
> Thanks Inki,
> 
> One more thing. mixer_layer_update is only called on for mixer version;
> MXR_VER_16_0_33_0, MXR_VER_128_0_0_184. This condition
> should have taken care of Exynos4 scenarios. What you say?
> 

There was my missing point. :) Already considered. ignore my comment.

Thanks,
Inki Dae

> Regards,
> Rahul Sharma.
> 
> On 24 June 2014 20:20, Inki Dae <inki.dae@samsung.com> wrote:
>> 2014-06-24 20:38 GMT+09:00 Andreas Färber <afaerber@suse.de>:
>>> Am 24.06.2014 07:21, schrieb Inki Dae:
>>>> On 2014? 06? 23? 14:32, Rahul Sharma wrote:
>>>>> Allowing only one layer update per vsync can cause issues
>>>>> while there are update available for both layers. There is
>>>>> a good amount of possibility to loose updates if we allow
>>>>> single update per vsync.
>>>>>
>>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos_mixer.c |    7 +------
>>>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> index d359501..6773b03 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
>>>>>  static void mixer_layer_update(struct mixer_context *ctx)
>>>>>  {
>>>>>      struct mixer_resources *res = &ctx->mixer_res;
>>>>> -    u32 val;
>>>>> -
>>>>> -    val = mixer_reg_read(res, MXR_CFG);
>>>>>
>>>>> -    /* allow one update per vsync only */
>>>>> -    if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
>>>>> -            mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>>> +    mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
>>>>
>>>> Rahul, it looks good to me and ok as is. But above codes don't consider
>>>> Exynos4 series. In case of Exynos4xxx SoC,
>>>> MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG
>>>> register are reserved fields. So can you work that patch to be
>>>> considered for Exynos4xxx SoC? That patch would be additional one.
>>>>
>>>> Anyway, will apply it as is, and I will wait for the additional patch.
>>>
>>> If it's not too late, could you fix up "multiple" in the subject? :)
>>
>> Corrected. :)
>>
>> Thanks,
>> Inki Dae
>>
>>>
>>> Cheers,
>>> Andreas
>>>
>>> --
>>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index d359501..6773b03 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -511,13 +511,8 @@  static void vp_video_buffer(struct mixer_context *ctx, int win)
 static void mixer_layer_update(struct mixer_context *ctx)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
-	u32 val;
-
-	val = mixer_reg_read(res, MXR_CFG);
 
-	/* allow one update per vsync only */
-	if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK))
-		mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
+	mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
 }
 
 static void mixer_graph_buffer(struct mixer_context *ctx, int win)