[PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Maxime Ripard]

The HVS has three FIFOs that can be assigned to a number of PixelValves
through a mux.

However, changing that FIFO requires that we disable and then enable the
pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
just the active ones.

Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index af3ee3dcdab6..01fa60844695 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -643,7 +643,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 		struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 		unsigned int matching_channels;
 
-		if (!crtc_state->active)
+		if (!crtc_state->enable)
 			continue;
 
 		/*
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] drm/vc4: crtc: Keep the previously assigned HVS FIFO

[Maxime Ripard]

The HVS FIFOs are currently assigned each time we have an atomic_check
for all the enabled CRTCs.

However, if we are running multiple outputs in parallel and we happen to
disable the first (by index) CRTC, we end up changing the assigned FIFO
of the second CRTC without disabling and reenabling the pixelvalve which
ends up in a stall and eventually a VBLANK timeout.

In order to fix this, we can create a special value for our assigned
channel to mark it as disabled, and if our CRTC already had an assigned
channel in its previous state, we keep on using it.

Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 13 ++++++++++---
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
 drivers/gpu/drm/vc4/vc4_kms.c  | 21 +++++++++++++++------
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index a393f93390a2..be754120faa8 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -852,11 +852,18 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc,
 
 void vc4_crtc_reset(struct drm_crtc *crtc)
 {
+	struct vc4_crtc_state *vc4_crtc_state;
+
 	if (crtc->state)
 		vc4_crtc_destroy_state(crtc, crtc->state);
-	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
-	if (crtc->state)
-		__drm_atomic_helper_crtc_reset(crtc, crtc->state);
+
+	vc4_crtc_state = kzalloc(sizeof(*vc4_crtc_state), GFP_KERNEL);
+	if (!vc4_crtc_state)
+		return;
+
+	vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
+	crtc->state = &vc4_crtc_state->base;
+	__drm_atomic_helper_crtc_reset(crtc, crtc->state);
 }
 
 static const struct drm_crtc_funcs vc4_crtc_funcs = {
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 8c8d96b6289f..2b13f2126f13 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -531,6 +531,7 @@ struct vc4_crtc_state {
 		unsigned int bottom;
 	} margins;
 };
+#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1)
 
 static inline struct vc4_crtc_state *
 to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 01fa60844695..f452dad50c22 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -616,7 +616,7 @@ static int
 vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct drm_crtc *crtc;
 	int i, ret;
 
@@ -629,6 +629,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 	 * modified.
 	 */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct drm_crtc_state *crtc_state;
 		if (!crtc->state->enable)
 			continue;
 
@@ -637,15 +638,23 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 			return PTR_ERR(crtc_state);
 	}
 
-	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
-		struct vc4_crtc_state *vc4_crtc_state =
-			to_vc4_crtc_state(crtc_state);
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		struct vc4_crtc_state *new_vc4_crtc_state =
+			to_vc4_crtc_state(new_crtc_state);
 		struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 		unsigned int matching_channels;
 
-		if (!crtc_state->enable)
+		if (old_crtc_state->enable && !new_crtc_state->enable)
+			new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
+
+		if (!new_crtc_state->enable)
 			continue;
 
+		if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) {
+			unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel);
+			continue;
+		}
+
 		/*
 		 * The problem we have to solve here is that we have
 		 * up to 7 encoders, connected to up to 6 CRTCs.
@@ -674,7 +683,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 		if (matching_channels) {
 			unsigned int channel = ffs(matching_channels) - 1;
 
-			vc4_crtc_state->assigned_channel = channel;
+			new_vc4_crtc_state->assigned_channel = channel;
 			unassigned_channels &= ~BIT(channel);
 		} else {
 			return -EINVAL;
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] drm/vc4: crtc: Keep the previously assigned HVS FIFO

[Dave Stevenson]

Hi Maxime

Thanks for the patch - it makes mode switching reliable for me! :-)

On Fri, 18 Sep 2020 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote:
>
> The HVS FIFOs are currently assigned each time we have an atomic_check
> for all the enabled CRTCs.
>
> However, if we are running multiple outputs in parallel and we happen to
> disable the first (by index) CRTC, we end up changing the assigned FIFO
> of the second CRTC without disabling and reenabling the pixelvalve which
> ends up in a stall and eventually a VBLANK timeout.
>
> In order to fix this, we can create a special value for our assigned
> channel to mark it as disabled, and if our CRTC already had an assigned
> channel in its previous state, we keep on using it.
>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Review comments below though.

> ---
>  drivers/gpu/drm/vc4/vc4_crtc.c | 13 ++++++++++---
>  drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
>  drivers/gpu/drm/vc4/vc4_kms.c  | 21 +++++++++++++++------
>  3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index a393f93390a2..be754120faa8 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -852,11 +852,18 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>
>  void vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> +       struct vc4_crtc_state *vc4_crtc_state;
> +
>         if (crtc->state)
>                 vc4_crtc_destroy_state(crtc, crtc->state);
> -       crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
> -       if (crtc->state)
> -               __drm_atomic_helper_crtc_reset(crtc, crtc->state);
> +
> +       vc4_crtc_state = kzalloc(sizeof(*vc4_crtc_state), GFP_KERNEL);
> +       if (!vc4_crtc_state)
> +               return;

This error path has me worried, but I may have missed it.

If crtc->state was set then it's been freed via
vc4_crtc_destroy_state. However I don't see anything that has reset
crtc->state to NULL.
If the new alloc fails then I believe we exit with crtc->state still
set to the old freed pointer.

The old code directly set crtc->state, so it got reset to NULL by the failure.

> +
> +       vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
> +       crtc->state = &vc4_crtc_state->base;
> +       __drm_atomic_helper_crtc_reset(crtc, crtc->state);
>  }
>
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 8c8d96b6289f..2b13f2126f13 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -531,6 +531,7 @@ struct vc4_crtc_state {
>                 unsigned int bottom;
>         } margins;
>  };
> +#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1)

Checkpatch whinge on that
CHECK: No space is necessary after a cast
#55: FILE: drivers/gpu/drm/vc4/vc4_drv.h:539:
+#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1)

CHECK: Please use a blank line after function/struct/union/enum declarations
#55: FILE: drivers/gpu/drm/vc4/vc4_drv.h:539:
 };
+#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1)

>  static inline struct vc4_crtc_state *
>  to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 01fa60844695..f452dad50c22 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -616,7 +616,7 @@ static int
>  vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>  {
>         unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
> -       struct drm_crtc_state *crtc_state;
> +       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>         struct drm_crtc *crtc;
>         int i, ret;
>
> @@ -629,6 +629,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>          * modified.
>          */
>         list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +               struct drm_crtc_state *crtc_state;

Blank line between variables and code, or not in this subsystem?
Checkpatch hasn't complained to me here.

>                 if (!crtc->state->enable)
>                         continue;
>
> @@ -637,15 +638,23 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>                         return PTR_ERR(crtc_state);
>         }
>
> -       for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> -               struct vc4_crtc_state *vc4_crtc_state =
> -                       to_vc4_crtc_state(crtc_state);
> +       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +               struct vc4_crtc_state *new_vc4_crtc_state =
> +                       to_vc4_crtc_state(new_crtc_state);
>                 struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>                 unsigned int matching_channels;
>
> -               if (!crtc_state->enable)
> +               if (old_crtc_state->enable && !new_crtc_state->enable)
> +                       new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
> +
> +               if (!new_crtc_state->enable)
>                         continue;
>
> +               if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) {
> +                       unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel);
> +                       continue;
> +               }
> +
>                 /*
>                  * The problem we have to solve here is that we have
>                  * up to 7 encoders, connected to up to 6 CRTCs.
> @@ -674,7 +683,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>                 if (matching_channels) {
>                         unsigned int channel = ffs(matching_channels) - 1;
>
> -                       vc4_crtc_state->assigned_channel = channel;
> +                       new_vc4_crtc_state->assigned_channel = channel;
>                         unassigned_channels &= ~BIT(channel);
>                 } else {
>                         return -EINVAL;
> --
> 2.26.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Dave Stevenson]

Hi Maxime

Thanks for the patch.

On Fri, 18 Sep 2020 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote:
>
> The HVS has three FIFOs that can be assigned to a number of PixelValves
> through a mux.
>
> However, changing that FIFO requires that we disable and then enable the
> pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> just the active ones.
>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
>  drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index af3ee3dcdab6..01fa60844695 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -643,7 +643,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>                 struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>                 unsigned int matching_channels;
>
> -               if (!crtc_state->active)
> +               if (!crtc_state->enable)
>                         continue;
>
>                 /*
> --
> 2.26.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 775 bytes --]

On Fri, Sep 18, 2020 at 04:43:20PM +0100, Dave Stevenson wrote:
> Hi Maxime
> 
> Thanks for the patch.
> 
> On Fri, 18 Sep 2020 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > The HVS has three FIFOs that can be assigned to a number of PixelValves
> > through a mux.
> >
> > However, changing that FIFO requires that we disable and then enable the
> > pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> > just the active ones.
> >
> > Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Applied, thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Hoegeun Kwon]

Hi Maxime,

On 9/18/20 11:59 PM, Maxime Ripard wrote:
> The HVS has three FIFOs that can be assigned to a number of PixelValves
> through a mux.
>
> However, changing that FIFO requires that we disable and then enable the
> pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> just the active ones.

Thanks for the patch.

There is a problem when doing page flip.
After connecting 2 HDMIs without hotplug and booting.(Same thing when
using hotplug.)

When executing page flip for each of HDMI 0 and 1 in modetest
operation does not work normally. (crtc irq does not occur, so timeout 
occurs.)
Sometimes both hdmi 0 or 1 work or not.

LOGs:
root:~> modetest -Mvc4 -s 32:1280x720 -v
setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64
failed to set gamma: Invalid argument
freq: 60.24Hz
freq: 60.00Hz

root:~> modetest -Mvc4 -s 38:1280x720 -v
setting mode 1280x720-60Hz@XR24 on connectors 38, crtc 69
failed to set gamma: Invalid argument
select timed out or error (ret 0)
select timed out or error (ret 0)

Could you please check it?

Best regards,
Hoegeun

>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index af3ee3dcdab6..01fa60844695 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -643,7 +643,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
>   		struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>   		unsigned int matching_channels;
>   
> -		if (!crtc_state->active)
> +		if (!crtc_state->enable)
>   			continue;
>   
>   		/*

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1272 bytes --]

On Thu, Sep 24, 2020 at 05:08:56PM +0900, Hoegeun Kwon wrote:
> Hi Maxime,
> 
> On 9/18/20 11:59 PM, Maxime Ripard wrote:
> > The HVS has three FIFOs that can be assigned to a number of PixelValves
> > through a mux.
> >
> > However, changing that FIFO requires that we disable and then enable the
> > pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> > just the active ones.
> 
> Thanks for the patch.
> 
> There is a problem when doing page flip.
> After connecting 2 HDMIs without hotplug and booting.(Same thing when
> using hotplug.)
> 
> When executing page flip for each of HDMI 0 and 1 in modetest
> operation does not work normally. (crtc irq does not occur, so timeout 
> occurs.)
> Sometimes both hdmi 0 or 1 work or not.
> 
> LOGs:
> root:~> modetest -Mvc4 -s 32:1280x720 -v
> setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64
> failed to set gamma: Invalid argument
> freq: 60.24Hz
> freq: 60.00Hz
> 
> root:~> modetest -Mvc4 -s 38:1280x720 -v
> setting mode 1280x720-60Hz@XR24 on connectors 38, crtc 69
> failed to set gamma: Invalid argument
> select timed out or error (ret 0)
> select timed out or error (ret 0)
> 
> Could you please check it?

I'll look into it, thanks :)

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2141 bytes --]

On Thu, Sep 24, 2020 at 05:08:56PM +0900, Hoegeun Kwon wrote:
> Hi Maxime,
> 
> On 9/18/20 11:59 PM, Maxime Ripard wrote:
> > The HVS has three FIFOs that can be assigned to a number of PixelValves
> > through a mux.
> >
> > However, changing that FIFO requires that we disable and then enable the
> > pixelvalve, so we want to assign FIFOs to all the enabled CRTCs, and not
> > just the active ones.
> 
> Thanks for the patch.
> 
> There is a problem when doing page flip.
> After connecting 2 HDMIs without hotplug and booting.(Same thing when
> using hotplug.)
> 
> When executing page flip for each of HDMI 0 and 1 in modetest
> operation does not work normally. (crtc irq does not occur, so timeout 
> occurs.)
> Sometimes both hdmi 0 or 1 work or not.
> 
> LOGs:
> root:~> modetest -Mvc4 -s 32:1280x720 -v
> setting mode 1280x720-60Hz@XR24 on connectors 32, crtc 64
> failed to set gamma: Invalid argument
> freq: 60.24Hz
> freq: 60.00Hz
> 
> root:~> modetest -Mvc4 -s 38:1280x720 -v
> setting mode 1280x720-60Hz@XR24 on connectors 38, crtc 69
> failed to set gamma: Invalid argument
> select timed out or error (ret 0)
> select timed out or error (ret 0)
> 
> Could you please check it?

So I can reproduce that bug, but I've not been able to fix it yet.

The issue seems to happen 100% of the time when you start first with the
second connector, and then the first. It creates yet another muxing
corner case, which is that when the first modeset runs, there's only one
enabled CRTC (with the 69 here) that gets assigned the channel 0 (since
it's the only one and it can run from that channel). However, when
modeset exits, it doesn't disable that CRTC for some reason.

Then you enable a second CRTC (64) with the second modeset command, but
it has a lower index so it gets evaluated first and gets assigned the
channel 0 as well since we haven't removed the CRTC 69 from the pool
yet.

I've fixed that up by first removing the channels in current use, and
then allocating them in two separate passes, but it doesn't address the
problem entirely, so I'll keep looking.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel