[v1,2/3] drm/tegra: plane: Correct legacy blending

Message ID 00cf4565238be471c90e5823e53e0ed504869bfe.1521075485.git.digetx@gmail.com
State Deferred
Headers show
Series
  • [v1,1/3] drm/tegra: plane: Fix RGB565 plane format on older Tegra's
Related show

Commit Message

Dmitry Osipenko March 15, 2018, 1 a.m.
Keep old 'dependent' state of unaffected planes, this way new state takes
into account current state of unaffected planes.

Fixes: ebae8d07435a ("drm/tegra: dc: Implement legacy blending")
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/plane.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Comments

Thierry Reding March 15, 2018, 10:29 a.m. | #1
On Thu, Mar 15, 2018 at 04:00:24AM +0300, Dmitry Osipenko wrote:
> Keep old 'dependent' state of unaffected planes, this way new state takes
> into account current state of unaffected planes.
> 
> Fixes: ebae8d07435a ("drm/tegra: dc: Implement legacy blending")
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/plane.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> index fc37dcf8c458..3c0cb6a04c66 100644
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
> @@ -287,13 +287,11 @@ unsigned int tegra_plane_format_adjust(unsigned int opaque)
>  	return opaque;
>  }
>  
> -unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
> -					   struct tegra_plane *other)
> +static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
> +						  struct tegra_plane *other)

I'd prefer this to be a separate patch to keep the diff down to make
this easier to apply to v4.16. I can do that when I apply, no need to
resend.

>  {
>  	unsigned int index = 0, i;
>  
> -	WARN_ON(plane == other);
> -

Why would this need to go away? We still shouldn't be called with plane
== other because that makes no sense.

Thierry
Dmitry Osipenko March 15, 2018, 12:42 p.m. | #2
On 15.03.2018 13:29, Thierry Reding wrote:
> On Thu, Mar 15, 2018 at 04:00:24AM +0300, Dmitry Osipenko wrote:
>> Keep old 'dependent' state of unaffected planes, this way new state takes
>> into account current state of unaffected planes.
>>
>> Fixes: ebae8d07435a ("drm/tegra: dc: Implement legacy blending")
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/gpu/drm/tegra/plane.c | 15 ++++++---------
>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
>> index fc37dcf8c458..3c0cb6a04c66 100644
>> --- a/drivers/gpu/drm/tegra/plane.c
>> +++ b/drivers/gpu/drm/tegra/plane.c
>> @@ -287,13 +287,11 @@ unsigned int tegra_plane_format_adjust(unsigned int opaque)
>>  	return opaque;
>>  }
>>  
>> -unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>> -					   struct tegra_plane *other)
>> +static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>> +						  struct tegra_plane *other)
> 
> I'd prefer this to be a separate patch to keep the diff down to make
> this easier to apply to v4.16. I can do that when I apply, no need to
> resend.

Okay. But now I'm thinking that it's probably not really worth to backport this
patch at all because it doesn't fix blending entirely, but only makes it good
enough to show cursor plane properly. I'll send V2.

>>  {
>>  	unsigned int index = 0, i;
>>  
>> -	WARN_ON(plane == other);
>> -
> 
> Why would this need to go away? We still shouldn't be called with plane
> == other because that makes no sense.
This can't ever happen because we are skipping 'plane' in for_each_plane_in_state().
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko March 15, 2018, 1:47 p.m. | #3
On 15.03.2018 15:42, Dmitry Osipenko wrote:
> On 15.03.2018 13:29, Thierry Reding wrote:
>> On Thu, Mar 15, 2018 at 04:00:24AM +0300, Dmitry Osipenko wrote:
>>> Keep old 'dependent' state of unaffected planes, this way new state takes
>>> into account current state of unaffected planes.
>>>
>>> Fixes: ebae8d07435a ("drm/tegra: dc: Implement legacy blending")
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/gpu/drm/tegra/plane.c | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
>>> index fc37dcf8c458..3c0cb6a04c66 100644
>>> --- a/drivers/gpu/drm/tegra/plane.c
>>> +++ b/drivers/gpu/drm/tegra/plane.c
>>> @@ -287,13 +287,11 @@ unsigned int tegra_plane_format_adjust(unsigned int opaque)
>>>  	return opaque;
>>>  }
>>>  
>>> -unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>>> -					   struct tegra_plane *other)
>>> +static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>>> +						  struct tegra_plane *other)
>>
>> I'd prefer this to be a separate patch to keep the diff down to make
>> this easier to apply to v4.16. I can do that when I apply, no need to
>> resend.
> 
> Okay. But now I'm thinking that it's probably not really worth to backport this
> patch at all because it doesn't fix blending entirely, but only makes it good
> enough to show cursor plane properly. I'll send V2.

Ah, didn't notice that I don't have to re-send. I've spotted other problem in
the patch, so will just send V2 ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko March 15, 2018, 1:47 p.m. | #4
On 15.03.2018 15:42, Dmitry Osipenko wrote:
> On 15.03.2018 13:29, Thierry Reding wrote:
>> On Thu, Mar 15, 2018 at 04:00:24AM +0300, Dmitry Osipenko wrote:
>>> Keep old 'dependent' state of unaffected planes, this way new state takes
>>> into account current state of unaffected planes.
>>>
>>> Fixes: ebae8d07435a ("drm/tegra: dc: Implement legacy blending")
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/gpu/drm/tegra/plane.c | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
>>> index fc37dcf8c458..3c0cb6a04c66 100644
>>> --- a/drivers/gpu/drm/tegra/plane.c
>>> +++ b/drivers/gpu/drm/tegra/plane.c
>>> @@ -287,13 +287,11 @@ unsigned int tegra_plane_format_adjust(unsigned int opaque)
>>>  	return opaque;
>>>  }
>>>  
>>> -unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>>> -					   struct tegra_plane *other)
>>> +static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
>>> +						  struct tegra_plane *other)
>>
>> I'd prefer this to be a separate patch to keep the diff down to make
>> this easier to apply to v4.16. I can do that when I apply, no need to
>> resend.
> 
> Okay. But now I'm thinking that it's probably not really worth to backport this
> patch at all because it doesn't fix blending entirely, but only makes it good
> enough to show cursor plane properly. I'll send V2.

Ah, didn't notice that I don't have to re-send. I've spotted other problem in
the patch, so will just send V2 ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index fc37dcf8c458..3c0cb6a04c66 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -287,13 +287,11 @@  unsigned int tegra_plane_format_adjust(unsigned int opaque)
 	return opaque;
 }
 
-unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
-					   struct tegra_plane *other)
+static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
+						  struct tegra_plane *other)
 {
 	unsigned int index = 0, i;
 
-	WARN_ON(plane == other);
-
 	for (i = 0; i < 3; i++) {
 		if (i == plane->index)
 			continue;
@@ -310,18 +308,15 @@  unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
 void tegra_plane_check_dependent(struct tegra_plane *tegra,
 				 struct tegra_plane_state *state)
 {
-	struct drm_plane_state *old, *new;
+	struct drm_plane_state *new;
 	struct drm_plane *plane;
 	unsigned int zpos[2];
 	unsigned int i;
 
-	for (i = 0; i < 3; i++)
-		state->dependent[i] = false;
-
 	for (i = 0; i < 2; i++)
 		zpos[i] = 0;
 
-	for_each_oldnew_plane_in_state(state->base.state, plane, old, new, i) {
+	for_each_new_plane_in_state(state->base.state, plane, new, i) {
 		struct tegra_plane *p = to_tegra_plane(plane);
 		unsigned index;
 
@@ -331,6 +326,8 @@  void tegra_plane_check_dependent(struct tegra_plane *tegra,
 
 		index = tegra_plane_get_overlap_index(tegra, p);
 
+		state->dependent[i] = false;
+
 		/*
 		 * If any of the other planes is on top of this plane and uses
 		 * a format with an alpha component, mark this plane as being