dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/omapdrm: Couple of plane related fixes
@ 2017-01-27 10:04 Jyri Sarha
  2017-01-27 10:04 ` [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs" Jyri Sarha
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jyri Sarha @ 2017-01-27 10:04 UTC (permalink / raw)
  To: dri-devel; +Cc: tomi.valkeinen, Jyri Sarha, laurent.pinchart

I hit these two problems while tracking scaling related problems with
omapdrm on DSS5 HW.

Jyri Sarha (2):
  Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive
    CRTCs"
  drm/omapdrm: Move commit_modeset_enables() before commit_planes()

 drivers/gpu/drm/omapdrm/omap_crtc.c |  8 +-------
 drivers/gpu/drm/omapdrm/omap_drv.c  | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 9 deletions(-)

-- 
1.9.1

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs"
  2017-01-27 10:04 [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Jyri Sarha
@ 2017-01-27 10:04 ` Jyri Sarha
  2017-01-28 16:11   ` Laurent Pinchart
  2017-01-27 10:04 ` [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes() Jyri Sarha
  2017-02-06 11:50 ` [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Tomi Valkeinen
  2 siblings, 1 reply; 11+ messages in thread
From: Jyri Sarha @ 2017-01-27 10:04 UTC (permalink / raw)
  To: dri-devel; +Cc: tomi.valkeinen, Jyri Sarha, laurent.pinchart

This reverts commit dadf4659d0608e034b6633f30300c2eff2dafb4c.

If planes are not disabled when the they are not on any crtc anymore
they will remain active and may show as "ghosts" when the crtc they
were last on is active again.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 8 +-------
 drivers/gpu/drm/omapdrm/omap_drv.c  | 3 +--
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index dd47dc1..b68c70e 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -410,13 +410,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 		dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
 	}
 
-	/*
-	 * Only flush the CRTC if it is currently enabled. CRTCs that require a
-	 * mode set are disabled prior plane updates and enabled afterwards.
-	 * They are thus not active (regardless of what their CRTC core state
-	 * reports) and the DRM core could thus call this function even though
-	 * the CRTC is currently disabled. Do nothing in that case.
-	 */
+	/* Only flush the CRTC if it is currently enabled. */
 	if (!omap_crtc->enabled)
 		return;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 00aa214..5e55f1b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -96,8 +96,7 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 	dispc_runtime_get();
 
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
-	drm_atomic_helper_commit_planes(dev, old_state,
-					DRM_PLANE_COMMIT_ACTIVE_ONLY);
+	drm_atomic_helper_commit_planes(dev, old_state, 0);
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
 
 	omap_atomic_wait_for_completion(dev, old_state);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-27 10:04 [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Jyri Sarha
  2017-01-27 10:04 ` [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs" Jyri Sarha
@ 2017-01-27 10:04 ` Jyri Sarha
  2017-01-28 16:17   ` Laurent Pinchart
  2017-02-06 11:50 ` [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Tomi Valkeinen
  2 siblings, 1 reply; 11+ messages in thread
From: Jyri Sarha @ 2017-01-27 10:04 UTC (permalink / raw)
  To: dri-devel; +Cc: tomi.valkeinen, Jyri Sarha, laurent.pinchart

Move drm_atomic_helper_commit_modeset_enables() call to before
drm_atomic_helper_commit_planes() call and have a
omap_atomic_wait_for_completion() call after both.

With the current dss dispc implementation we have to enable the new
modeset before we can commit planes. The dispc ovl configuration
relies on the video mode configuration been written into the HW when
the ovl configuration is calculated.

This approach is not ideal because after a mode change the plane
update is executed only after the first vblank interrupt. The dispc
implementation should be fixed so that it is able use uncommitted drm
state information.  information.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 5e55f1b..8c15735 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -96,8 +96,22 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
 	dispc_runtime_get();
 
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
-	drm_atomic_helper_commit_planes(dev, old_state, 0);
+
+	/* With the current dss dispc implementation we have to enable
+	 * the new modeset before we can commit planes. The dispc ovl
+	 * configuration relies on the video mode configuration been
+	 * written into the HW when the ovl configuration is
+	 * calculated.
+	 *
+	 * This approach is not ideal because after a mode change the
+	 * plane update is executed only after the first vblank
+	 * interrupt. The dispc implementation should be fixed so that
+	 * it is able use uncommitted drm state information.
+	 */
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
+	omap_atomic_wait_for_completion(dev, old_state);
+
+	drm_atomic_helper_commit_planes(dev, old_state, 0);
 
 	omap_atomic_wait_for_completion(dev, old_state);
 
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs"
  2017-01-27 10:04 ` [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs" Jyri Sarha
@ 2017-01-28 16:11   ` Laurent Pinchart
  2017-01-30  8:48     ` Tomi Valkeinen
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2017-01-28 16:11 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: tomi.valkeinen, dri-devel

Hi Jyri,

Thank you for the patch.

On Friday 27 Jan 2017 12:04:54 Jyri Sarha wrote:
> This reverts commit dadf4659d0608e034b6633f30300c2eff2dafb4c.
> 
> If planes are not disabled when the they are not on any crtc anymore
> they will remain active and may show as "ghosts" when the crtc they
> were last on is active again.

Sorry for the breakage.

The drm_atomic_helper_commit_planes() helper documentation states

 * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
 * @flags in order not to receive plane update notifications related to a
 * disabled CRTC. This avoids the need to manually ignore plane updates in
 * driver code when the driver and/or hardware can't or just don't need to
 * deal with updates on disabled CRTCs, for example when supporting runtime
 * PM.

I wonder what this implies when CRTCs are being disabled. I see very few cases 
where the hardware wouldn't need the plane atomic disable operation being 
called when a plane is being disabled due to its CRTC being disabled. Maybe 
this should thus be addressed in the core. Daniel, any comment on this ?

> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 8 +-------
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 3 +--
>  2 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index dd47dc1..b68c70e 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -410,13 +410,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
>  	}
> 
> -	/*
> -	 * Only flush the CRTC if it is currently enabled. CRTCs that require 
a
> -	 * mode set are disabled prior plane updates and enabled afterwards.
> -	 * They are thus not active (regardless of what their CRTC core state
> -	 * reports) and the DRM core could thus call this function even though
> -	 * the CRTC is currently disabled. Do nothing in that case.
> -	 */
> +	/* Only flush the CRTC if it is currently enabled. */
>  	if (!omap_crtc->enabled)
>  		return;
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 00aa214..5e55f1b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -96,8 +96,7 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) dispc_runtime_get();
> 
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> -	drm_atomic_helper_commit_planes(dev, old_state,
> -					DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +	drm_atomic_helper_commit_planes(dev, old_state, 0);
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> 
>  	omap_atomic_wait_for_completion(dev, old_state);

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-27 10:04 ` [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes() Jyri Sarha
@ 2017-01-28 16:17   ` Laurent Pinchart
  2017-01-30  8:50     ` Tomi Valkeinen
  2017-01-30 11:11     ` Jyri Sarha
  0 siblings, 2 replies; 11+ messages in thread
From: Laurent Pinchart @ 2017-01-28 16:17 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: tomi.valkeinen, dri-devel

Hi Jyri,

Thank you for the patch.

On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote:
> Move drm_atomic_helper_commit_modeset_enables() call to before
> drm_atomic_helper_commit_planes() call and have a
> omap_atomic_wait_for_completion() call after both.
> 
> With the current dss dispc implementation we have to enable the new
> modeset before we can commit planes. The dispc ovl configuration
> relies on the video mode configuration been written into the HW when
> the ovl configuration is calculated.
> 
> This approach is not ideal because after a mode change the plane
> update is executed only after the first vblank interrupt. The dispc
> implementation should be fixed so that it is able use uncommitted drm
> state information.  information.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) dispc_runtime_get();
> 
>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> -	drm_atomic_helper_commit_planes(dev, old_state, 0);
> +
> +	/* With the current dss dispc implementation we have to enable
> +	 * the new modeset before we can commit planes. The dispc ovl
> +	 * configuration relies on the video mode configuration been
> +	 * written into the HW when the ovl configuration is
> +	 * calculated.
> +	 *
> +	 * This approach is not ideal because after a mode change the
> +	 * plane update is executed only after the first vblank
> +	 * interrupt. The dispc implementation should be fixed so that
> +	 * it is able use uncommitted drm state information.

Any chance you could fix the dispc implementation ? ;-) Otherwise, could you 
rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and 
zpos" ?

> +	 */
>  	drm_atomic_helper_commit_modeset_enables(dev, old_state);
> +	omap_atomic_wait_for_completion(dev, old_state);
> +
> +	drm_atomic_helper_commit_planes(dev, old_state, 0);
> 
>  	omap_atomic_wait_for_completion(dev, old_state);

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs"
  2017-01-28 16:11   ` Laurent Pinchart
@ 2017-01-30  8:48     ` Tomi Valkeinen
  0 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2017-01-30  8:48 UTC (permalink / raw)
  To: Laurent Pinchart, Jyri Sarha; +Cc: dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1415 bytes --]

On 28/01/17 18:11, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Friday 27 Jan 2017 12:04:54 Jyri Sarha wrote:
>> This reverts commit dadf4659d0608e034b6633f30300c2eff2dafb4c.
>>
>> If planes are not disabled when the they are not on any crtc anymore
>> they will remain active and may show as "ghosts" when the crtc they
>> were last on is active again.
> 
> Sorry for the breakage.
> 
> The drm_atomic_helper_commit_planes() helper documentation states
> 
>  * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
>  * @flags in order not to receive plane update notifications related to a
>  * disabled CRTC. This avoids the need to manually ignore plane updates in
>  * driver code when the driver and/or hardware can't or just don't need to
>  * deal with updates on disabled CRTCs, for example when supporting runtime
>  * PM.
> 
> I wonder what this implies when CRTCs are being disabled. I see very few cases 
> where the hardware wouldn't need the plane atomic disable operation being 
> called when a plane is being disabled due to its CRTC being disabled. Maybe 
> this should thus be addressed in the core. Daniel, any comment on this ?

Similar change was done to mali in "drm: mali-dp: Don't set
DRM_PLANE_COMMIT_ACTIVE_ONLY", so at least omapdrm is not alone here. I
also wonder if this behavior is correct.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-28 16:17   ` Laurent Pinchart
@ 2017-01-30  8:50     ` Tomi Valkeinen
  2017-01-30 11:11     ` Jyri Sarha
  1 sibling, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2017-01-30  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Jyri Sarha; +Cc: dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2330 bytes --]

On 28/01/17 18:17, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote:
>> Move drm_atomic_helper_commit_modeset_enables() call to before
>> drm_atomic_helper_commit_planes() call and have a
>> omap_atomic_wait_for_completion() call after both.
>>
>> With the current dss dispc implementation we have to enable the new
>> modeset before we can commit planes. The dispc ovl configuration
>> relies on the video mode configuration been written into the HW when
>> the ovl configuration is calculated.
>>
>> This approach is not ideal because after a mode change the plane
>> update is executed only after the first vblank interrupt. The dispc
>> implementation should be fixed so that it is able use uncommitted drm
>> state information.  information.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
>> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct
>> omap_atomic_state_commit *commit) dispc_runtime_get();
>>
>>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> -	drm_atomic_helper_commit_planes(dev, old_state, 0);
>> +
>> +	/* With the current dss dispc implementation we have to enable
>> +	 * the new modeset before we can commit planes. The dispc ovl
>> +	 * configuration relies on the video mode configuration been
>> +	 * written into the HW when the ovl configuration is
>> +	 * calculated.
>> +	 *
>> +	 * This approach is not ideal because after a mode change the
>> +	 * plane update is executed only after the first vblank
>> +	 * interrupt. The dispc implementation should be fixed so that
>> +	 * it is able use uncommitted drm state information.
> 
> Any chance you could fix the dispc implementation ? ;-) Otherwise, could you 

Dispc change is on our todo, but the size of the change won't fit into
"fix" definition =). There's a lot of state that the dispc side is
looking at, which at the moment doesn't come from drm side.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-28 16:17   ` Laurent Pinchart
  2017-01-30  8:50     ` Tomi Valkeinen
@ 2017-01-30 11:11     ` Jyri Sarha
  2017-01-30 11:15       ` Laurent Pinchart
  1 sibling, 1 reply; 11+ messages in thread
From: Jyri Sarha @ 2017-01-30 11:11 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: tomi.valkeinen, dri-devel

On 01/28/17 18:17, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote:
>> Move drm_atomic_helper_commit_modeset_enables() call to before
>> drm_atomic_helper_commit_planes() call and have a
>> omap_atomic_wait_for_completion() call after both.
>>
>> With the current dss dispc implementation we have to enable the new
>> modeset before we can commit planes. The dispc ovl configuration
>> relies on the video mode configuration been written into the HW when
>> the ovl configuration is calculated.
>>
>> This approach is not ideal because after a mode change the plane
>> update is executed only after the first vblank interrupt. The dispc
>> implementation should be fixed so that it is able use uncommitted drm
>> state information.  information.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
>> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct
>> omap_atomic_state_commit *commit) dispc_runtime_get();
>>
>>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
>> -	drm_atomic_helper_commit_planes(dev, old_state, 0);
>> +
>> +	/* With the current dss dispc implementation we have to enable
>> +	 * the new modeset before we can commit planes. The dispc ovl
>> +	 * configuration relies on the video mode configuration been
>> +	 * written into the HW when the ovl configuration is
>> +	 * calculated.
>> +	 *
>> +	 * This approach is not ideal because after a mode change the
>> +	 * plane update is executed only after the first vblank
>> +	 * interrupt. The dispc implementation should be fixed so that
>> +	 * it is able use uncommitted drm state information.
> Any chance you could fix the dispc implementation ? ;-) Otherwise, could you 

I am currently studying how to do it, but it tricky... really tricky. In
DRM terms, the plane configuration is affected by the connector where
the crtc is connected to. The whole DSS should have to made understand
drm state structures. There certainly wont be any "fix" for 4.10,
probably nothing for 4.11 either.

> rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and 
> zpos" ?
> 

Thanks, I'll do that.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-30 11:11     ` Jyri Sarha
@ 2017-01-30 11:15       ` Laurent Pinchart
  2017-01-30 12:50         ` Jyri Sarha
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2017-01-30 11:15 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: tomi.valkeinen, dri-devel

Hi Jyri,

On Monday 30 Jan 2017 13:11:56 Jyri Sarha wrote:
> On 01/28/17 18:17, Laurent Pinchart wrote:
> > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote:
> >> Move drm_atomic_helper_commit_modeset_enables() call to before
> >> drm_atomic_helper_commit_planes() call and have a
> >> omap_atomic_wait_for_completion() call after both.
> >> 
> >> With the current dss dispc implementation we have to enable the new
> >> modeset before we can commit planes. The dispc ovl configuration
> >> relies on the video mode configuration been written into the HW when
> >> the ovl configuration is calculated.
> >> 
> >> This approach is not ideal because after a mode change the plane
> >> update is executed only after the first vblank interrupt. The dispc
> >> implementation should be fixed so that it is able use uncommitted drm
> >> state information.  information.
> >> 
> >> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++-
> >>  1 file changed, 15 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644
> >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct
> >> omap_atomic_state_commit *commit) dispc_runtime_get();
> >> 
> >>  	drm_atomic_helper_commit_modeset_disables(dev, old_state);
> >> 
> >> -	drm_atomic_helper_commit_planes(dev, old_state, 0);
> >> +
> >> +	/* With the current dss dispc implementation we have to enable
> >> +	 * the new modeset before we can commit planes. The dispc ovl
> >> +	 * configuration relies on the video mode configuration been
> >> +	 * written into the HW when the ovl configuration is
> >> +	 * calculated.
> >> +	 *
> >> +	 * This approach is not ideal because after a mode change the
> >> +	 * plane update is executed only after the first vblank
> >> +	 * interrupt. The dispc implementation should be fixed so that
> >> +	 * it is able use uncommitted drm state information.
> > 
> > Any chance you could fix the dispc implementation ? ;-) Otherwise, could
> > you
>
> I am currently studying how to do it, but it tricky... really tricky. In
> DRM terms, the plane configuration is affected by the connector where
> the crtc is connected to. The whole DSS should have to made understand
> drm state structures. There certainly wont be any "fix" for 4.10,
> probably nothing for 4.11 either.
> 
> > rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and
> > zpos" ?
> 
> Thanks, I'll do that.

If you intend on merging this patch as a v4.10 fix then there's no need to 
rebase. If it targets v4.11, the above-mentioned series will likely go in 
first.

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
  2017-01-30 11:15       ` Laurent Pinchart
@ 2017-01-30 12:50         ` Jyri Sarha
  0 siblings, 0 replies; 11+ messages in thread
From: Jyri Sarha @ 2017-01-30 12:50 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: tomi.valkeinen, dri-devel

On 01/30/17 13:15, Laurent Pinchart wrote:
>>> rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and
>>> zpos" ?
>> Thanks, I'll do that.
> If you intend on merging this patch as a v4.10 fix then there's no need to 
> rebase. If it targets v4.11, the above-mentioned series will likely go in 
> first.

Well perhaps this patch should go to v4.10 already. Because of the bug a
plane commit may fail if the screen is blanked and there is no pclk
available at the time the plane HW is configured.

Tomi, what do you think?

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] drm/omapdrm: Couple of plane related fixes
  2017-01-27 10:04 [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Jyri Sarha
  2017-01-27 10:04 ` [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs" Jyri Sarha
  2017-01-27 10:04 ` [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes() Jyri Sarha
@ 2017-02-06 11:50 ` Tomi Valkeinen
  2 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2017-02-06 11:50 UTC (permalink / raw)
  To: Jyri Sarha, dri-devel; +Cc: laurent.pinchart


[-- Attachment #1.1.1: Type: text/plain, Size: 797 bytes --]

Hi,

On 27/01/17 12:04, Jyri Sarha wrote:
> I hit these two problems while tracking scaling related problems with
> omapdrm on DSS5 HW.
> 
> Jyri Sarha (2):
>   Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive
>     CRTCs"
>   drm/omapdrm: Move commit_modeset_enables() before commit_planes()
> 
>  drivers/gpu/drm/omapdrm/omap_crtc.c |  8 +-------
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 17 +++++++++++++++--
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 

Both look good, I'll queue them for 4.11. I'm not sending them as fixes
to 4.10, as afaik they only affect atomic modesetting which is not that
widely used, and we're quite late in the -rc cycle. And the latter one
might well cause some regressions that we haven't found out...

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-02-06 11:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27 10:04 [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Jyri Sarha
2017-01-27 10:04 ` [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs" Jyri Sarha
2017-01-28 16:11   ` Laurent Pinchart
2017-01-30  8:48     ` Tomi Valkeinen
2017-01-27 10:04 ` [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes() Jyri Sarha
2017-01-28 16:17   ` Laurent Pinchart
2017-01-30  8:50     ` Tomi Valkeinen
2017-01-30 11:11     ` Jyri Sarha
2017-01-30 11:15       ` Laurent Pinchart
2017-01-30 12:50         ` Jyri Sarha
2017-02-06 11:50 ` [PATCH 0/2] drm/omapdrm: Couple of plane related fixes Tomi Valkeinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).