Re: [PATCH v4 14/78] drm/vc4: crtc: Assign output to channel automatically
Hi Maxime
On Wed, 8 Jul 2020 at 18:42, Maxime Ripard wrote:
>
> The HVS found in the BCM2711 has 6 outputs and 3 FIFOs, with each output
> being connected to a pixelvalve, and some muxing between the FIFOs and
> outputs.
>
> Any output cannot feed from any FIFO though, and they all have a bunch of
> constraints.
>
> In order to support this, let's store the possible FIFOs each output can be
> assigned to in the vc4_crtc_data, and use that information at atomic_check
> time to iterate over all the CRTCs enabled and assign them FIFOs.
>
> The channel assigned is then set in the vc4_crtc_state so that the rest of
> the driver can use it.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 12 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 7 +-
> drivers/gpu/drm/vc4/vc4_hvs.c | 28 ++
> drivers/gpu/drm/vc4/vc4_kms.c | 167 +-
> drivers/gpu/drm/vc4/vc4_regs.h | 10 ++-
> drivers/gpu/drm/vc4/vc4_txp.c | 1 +-
> 6 files changed, 199 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index fe2e5675aed4..b7e47ce1476c 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -88,6 +88,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> + struct vc4_crtc_state *vc4_crtc_state =
> to_vc4_crtc_state(crtc->state);
> unsigned int cob_size;
> u32 val;
> int fifo_lines;
> @@ -104,7 +105,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> * Read vertical scanline which is currently composed for our
> * pixelvalve by the HVS, and also the scaler status.
> */
> - val = HVS_READ(SCALER_DISPSTATX(vc4_crtc->channel));
> + val = HVS_READ(SCALER_DISPSTATX(vc4_crtc_state->assigned_channel));
>
> /* Get optional system timestamp after query. */
> if (etime)
> @@ -124,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
> *crtc,
> *hpos += mode->crtc_htotal / 2;
> }
>
> - cob_size = vc4_crtc_get_cob_allocation(vc4, vc4_crtc->channel);
> + cob_size = vc4_crtc_get_cob_allocation(vc4,
> vc4_crtc_state->assigned_channel);
> /* This is the offset we need for translating hvs -> pv scanout pos.
> */
> fifo_lines = cob_size / mode->crtc_hdisplay;
>
> @@ -520,7 +521,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc
> *vc4_crtc)
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
> - u32 chan = vc4_crtc->channel;
> + u32 chan = vc4_state->assigned_channel;
> unsigned long flags;
>
> spin_lock_irqsave(&dev->event_lock, flags);
> @@ -719,6 +720,7 @@ struct drm_crtc_state *vc4_crtc_duplicate_state(struct
> drm_crtc *crtc)
> old_vc4_state = to_vc4_crtc_state(crtc->state);
> vc4_state->feed_txp = old_vc4_state->feed_txp;
> vc4_state->margins = old_vc4_state->margins;
> + vc4_state->assigned_channel = old_vc4_state->assigned_channel;
>
> __drm_atomic_helper_crtc_duplicate_state(crtc, &vc4_state->base);
> return &vc4_state->base;
> @@ -779,6 +781,7 @@ static const struct drm_crtc_helper_funcs
> vc4_crtc_helper_funcs = {
>
> static const struct vc4_pv_data bcm2835_pv0_data = {
> .base = {
> + .hvs_available_channels = BIT(0),
> .hvs_output = 0,
> },
> .debugfs_name = "crtc0_regs",
> @@ -791,6 +794,7 @@ static const struct vc4_pv_data bcm2835_pv0_data = {
>
> static const struct vc4_pv_data bcm2835_pv1_data = {
> .base = {
> + .hvs_available_channels = BIT(2),
> .hvs_output = 2,
> },
> .debugfs_name = "crtc1_regs",
> @@ -803,6 +807,7 @@ static const struct vc4_pv_data bcm2835_pv1_data = {
>
> static const struct vc4_pv_data bcm2835_pv2_data = {
> .base = {
> + .hvs_available_channels = BIT(1),
> .hvs_output = 1,
> },
> .debugfs_name = "crtc2_regs",
> @@ -866,7 +871,6 @@ int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc
> *vc4_crtc,
> drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
> crtc_funcs, NULL);
> drm_crtc_helper_add(crtc, crtc_helper_funcs);
> - vc4_crtc->channel = vc4_crtc->data->hvs_output;
> drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r));
> drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size);
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index d1cf4c038180..9e81ad8331f1 100644
> --- a/d
[PATCH v4 14/78] drm/vc4: crtc: Assign output to channel automatically
The HVS found in the BCM2711 has 6 outputs and 3 FIFOs, with each output
being connected to a pixelvalve, and some muxing between the FIFOs and
outputs.
Any output cannot feed from any FIFO though, and they all have a bunch of
constraints.
In order to support this, let's store the possible FIFOs each output can be
assigned to in the vc4_crtc_data, and use that information at atomic_check
time to iterate over all the CRTCs enabled and assign them FIFOs.
The channel assigned is then set in the vc4_crtc_state so that the rest of
the driver can use it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 12 +-
drivers/gpu/drm/vc4/vc4_drv.h | 7 +-
drivers/gpu/drm/vc4/vc4_hvs.c | 28 ++
drivers/gpu/drm/vc4/vc4_kms.c | 167 +-
drivers/gpu/drm/vc4/vc4_regs.h | 10 ++-
drivers/gpu/drm/vc4/vc4_txp.c | 1 +-
6 files changed, 199 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index fe2e5675aed4..b7e47ce1476c 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -88,6 +88,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
*crtc,
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+ struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(crtc->state);
unsigned int cob_size;
u32 val;
int fifo_lines;
@@ -104,7 +105,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
*crtc,
* Read vertical scanline which is currently composed for our
* pixelvalve by the HVS, and also the scaler status.
*/
- val = HVS_READ(SCALER_DISPSTATX(vc4_crtc->channel));
+ val = HVS_READ(SCALER_DISPSTATX(vc4_crtc_state->assigned_channel));
/* Get optional system timestamp after query. */
if (etime)
@@ -124,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc
*crtc,
*hpos += mode->crtc_htotal / 2;
}
- cob_size = vc4_crtc_get_cob_allocation(vc4, vc4_crtc->channel);
+ cob_size = vc4_crtc_get_cob_allocation(vc4,
vc4_crtc_state->assigned_channel);
/* This is the offset we need for translating hvs -> pv scanout pos. */
fifo_lines = cob_size / mode->crtc_hdisplay;
@@ -520,7 +521,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc
*vc4_crtc)
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
- u32 chan = vc4_crtc->channel;
+ u32 chan = vc4_state->assigned_channel;
unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags);
@@ -719,6 +720,7 @@ struct drm_crtc_state *vc4_crtc_duplicate_state(struct
drm_crtc *crtc)
old_vc4_state = to_vc4_crtc_state(crtc->state);
vc4_state->feed_txp = old_vc4_state->feed_txp;
vc4_state->margins = old_vc4_state->margins;
+ vc4_state->assigned_channel = old_vc4_state->assigned_channel;
__drm_atomic_helper_crtc_duplicate_state(crtc, &vc4_state->base);
return &vc4_state->base;
@@ -779,6 +781,7 @@ static const struct drm_crtc_helper_funcs
vc4_crtc_helper_funcs = {
static const struct vc4_pv_data bcm2835_pv0_data = {
.base = {
+ .hvs_available_channels = BIT(0),
.hvs_output = 0,
},
.debugfs_name = "crtc0_regs",
@@ -791,6 +794,7 @@ static const struct vc4_pv_data bcm2835_pv0_data = {
static const struct vc4_pv_data bcm2835_pv1_data = {
.base = {
+ .hvs_available_channels = BIT(2),
.hvs_output = 2,
},
.debugfs_name = "crtc1_regs",
@@ -803,6 +807,7 @@ static const struct vc4_pv_data bcm2835_pv1_data = {
static const struct vc4_pv_data bcm2835_pv2_data = {
.base = {
+ .hvs_available_channels = BIT(1),
.hvs_output = 1,
},
.debugfs_name = "crtc2_regs",
@@ -866,7 +871,6 @@ int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc
*vc4_crtc,
drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
crtc_funcs, NULL);
drm_crtc_helper_add(crtc, crtc_helper_funcs);
- vc4_crtc->channel = vc4_crtc->data->hvs_output;
drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r));
drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index d1cf4c038180..9e81ad8331f1 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -447,6 +447,9 @@ to_vc4_encoder(struct drm_encoder *encoder)
}
struct vc4_crtc_data {
+ /* Bitmask of channels (FIFOs) of the HVS that the output can source
from */
+ unsigned int hvs_available_channels;
+
/* Whic
