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

2020-10-02 Thread Maxime Ripard
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


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2020-10-01 Thread Maxime Ripard
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


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2020-09-24 Thread 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 
> ---
>   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;
>   
>   /*
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2020-09-24 Thread Maxime Ripard
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  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 
> 
> Tested-by: Dave Stevenson 
> Reviewed-by: Dave Stevenson 

Applied, thanks!
Maxime


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

2020-09-19 Thread 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 
---
 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

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


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

2020-09-18 Thread Dave Stevenson
Hi Maxime

Thanks for the patch.

On Fri, 18 Sep 2020 at 15:59, 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.
>
> Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
> Signed-off-by: Maxime Ripard 

Tested-by: Dave Stevenson 
Reviewed-by: Dave Stevenson 

> ---
>  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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel