| Message ID | 20200918145918.101068-1-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active | expand |
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
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; > > /*
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
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
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; /*
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(-)