Hi,
Here's some patches to enable the HDR output in the RPi4/BCM2711 HDMI controller.
Let me know what you think, Maxime
Changes from v6: - Addressed the issues pointed out by Dave - Rebased on current drm-misc-next
Changes from v5: - Fixed the connector->state access in the connector state reset - Added the tags from Thomas and Dave
Changes from v4: - Added the tags from Thomas - Fixed an issue with the clock doubling - Changed a comment to match the code being introduced
Changes from v3: - Don't dereference the connector->state pointer if kzalloc failed
Changes from v2: - Rebased on current drm-misc-next - Fixed a bug that was dropping the refresh rate when the bpc count was increased
Changes from v1: - Added the coccinelle script to the first patch - Fixed the pixel_rate ramp up
Maxime Ripard (9): drm/vc4: hvs: Align the HVS atomic hooks to the new API drm/vc4: Pass the atomic state to encoder hooks drm/vc4: hdmi: Take into account the clock doubling flag in atomic_check drm/vc4: hdmi: Don't access the connector state in reset if kmalloc fails drm/vc4: hdmi: Create a custom connector state drm/vc4: hdmi: Store pixel frequency in the connector state drm/vc4: hdmi: Use the connector state pixel rate for the PHY drm/vc4: hdmi: Limit the BCM2711 to the max without scrambling drm/vc4: hdmi: Enable 10/12 bpc output
drivers/gpu/drm/vc4/vc4_crtc.c | 24 ++--- drivers/gpu/drm/vc4/vc4_drv.h | 19 ++-- drivers/gpu/drm/vc4/vc4_hdmi.c | 155 +++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 23 +++-- drivers/gpu/drm/vc4/vc4_hdmi_phy.c | 8 +- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 9 ++ drivers/gpu/drm/vc4/vc4_hvs.c | 18 ++-- drivers/gpu/drm/vc4/vc4_txp.c | 10 +- 8 files changed, 208 insertions(+), 58 deletions(-)
Since the CRTC setup in vc4 is split between the PixelValves/TXP and the HVS, only the PV/TXP atomic hooks were updated in the previous commits, but it makes sense to update the HVS ones too.
Reviewed-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++---- drivers/gpu/drm/vc4/vc4_drv.h | 9 ++++----- drivers/gpu/drm/vc4/vc4_hvs.c | 18 ++++++++++-------- drivers/gpu/drm/vc4/vc4_txp.c | 10 +++------- 4 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index ee4f657a5ce0..95b2dd9d7a38 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -503,8 +503,6 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { - struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, - crtc); struct drm_device *dev = crtc->dev; struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc); @@ -517,7 +515,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, */ drm_crtc_vblank_on(crtc);
- vc4_hvs_atomic_enable(crtc, old_state); + vc4_hvs_atomic_enable(crtc, state);
if (vc4_encoder->pre_crtc_configure) vc4_encoder->pre_crtc_configure(encoder); @@ -593,7 +591,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc, struct drm_connector_state *conn_state; int ret, i;
- ret = vc4_hvs_atomic_check(crtc, crtc_state); + ret = vc4_hvs_atomic_check(crtc, state); if (ret) return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 6db4c944ecba..5a115bc62cc8 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -913,11 +913,10 @@ void vc4_irq_reset(struct drm_device *dev); extern struct platform_driver vc4_hvs_driver; void vc4_hvs_stop_channel(struct drm_device *dev, unsigned int output); int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output); -int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state); -void vc4_hvs_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state); -void vc4_hvs_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_state); -void vc4_hvs_atomic_flush(struct drm_crtc *crtc, - struct drm_atomic_state *state); +int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state); +void vc4_hvs_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state); +void vc4_hvs_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state); +void vc4_hvs_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *state); void vc4_hvs_dump_state(struct drm_device *dev); void vc4_hvs_unmask_underrun(struct drm_device *dev, int channel); void vc4_hvs_mask_underrun(struct drm_device *dev, int channel); diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index cccd341e5d67..2b3a597fa65f 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -326,10 +326,10 @@ void vc4_hvs_stop_channel(struct drm_device *dev, unsigned int chan) SCALER_DISPSTATX_EMPTY); }
-int vc4_hvs_atomic_check(struct drm_crtc *crtc, - struct drm_crtc_state *state) +int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state) { - struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state); + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state); struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_plane *plane; @@ -341,10 +341,10 @@ int vc4_hvs_atomic_check(struct drm_crtc *crtc, /* The pixelvalve can only feed one encoder (and encoders are * 1:1 with connectors.) */ - if (hweight32(state->connector_mask) > 1) + if (hweight32(crtc_state->connector_mask) > 1) return -EINVAL;
- drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, state) + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, crtc_state) dlist_count += vc4_plane_dlist_size(plane_state);
dlist_count++; /* Account for SCALER_CTL0_END. */ @@ -391,11 +391,12 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc) }
void vc4_hvs_atomic_enable(struct drm_crtc *crtc, - struct drm_crtc_state *old_state) + struct drm_atomic_state *state) { 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); + struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(new_crtc_state); struct drm_display_mode *mode = &crtc->state->adjusted_mode; bool oneshot = vc4_state->feed_txp;
@@ -404,9 +405,10 @@ void vc4_hvs_atomic_enable(struct drm_crtc *crtc, }
void vc4_hvs_atomic_disable(struct drm_crtc *crtc, - struct drm_crtc_state *old_state) + struct drm_atomic_state *state) { struct drm_device *dev = crtc->dev; + struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc); struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(old_state); unsigned int chan = vc4_state->assigned_channel;
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 8aa5220885f4..0f4e7bee4255 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -395,7 +395,7 @@ static int vc4_txp_atomic_check(struct drm_crtc *crtc, struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state); int ret;
- ret = vc4_hvs_atomic_check(crtc, crtc_state); + ret = vc4_hvs_atomic_check(crtc, state); if (ret) return ret;
@@ -408,23 +408,19 @@ static int vc4_txp_atomic_check(struct drm_crtc *crtc, static void vc4_txp_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { - struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, - crtc); drm_crtc_vblank_on(crtc); - vc4_hvs_atomic_enable(crtc, old_state); + vc4_hvs_atomic_enable(crtc, state); }
static void vc4_txp_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state) { - struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, - crtc); struct drm_device *dev = crtc->dev;
/* Disable vblank irq handling before crtc is disabled. */ drm_crtc_vblank_off(crtc);
- vc4_hvs_atomic_disable(crtc, old_state); + vc4_hvs_atomic_disable(crtc, state);
/* * Make sure we issue a vblank event after disabling the CRTC if
We'll need to access the connector state in our encoder setup, so let's just pass the whole DRM state to our private encoder hooks.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 18 ++++++++++-------- drivers/gpu/drm/vc4/vc4_drv.h | 10 +++++----- drivers/gpu/drm/vc4/vc4_hdmi.c | 15 ++++++++++----- 3 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 95b2dd9d7a38..0fef45b93ba1 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -403,7 +403,9 @@ static void require_hvs_enabled(struct drm_device *dev) SCALER_DISPCTRL_ENABLE); }
-static int vc4_crtc_disable(struct drm_crtc *crtc, unsigned int channel) +static int vc4_crtc_disable(struct drm_crtc *crtc, + struct drm_atomic_state *state, + unsigned int channel) { struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc); struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); @@ -435,13 +437,13 @@ static int vc4_crtc_disable(struct drm_crtc *crtc, unsigned int channel) mdelay(20);
if (vc4_encoder && vc4_encoder->post_crtc_disable) - vc4_encoder->post_crtc_disable(encoder); + vc4_encoder->post_crtc_disable(encoder, state);
vc4_crtc_pixelvalve_reset(crtc); vc4_hvs_stop_channel(dev, channel);
if (vc4_encoder && vc4_encoder->post_crtc_powerdown) - vc4_encoder->post_crtc_powerdown(encoder); + vc4_encoder->post_crtc_powerdown(encoder, state);
return 0; } @@ -468,7 +470,7 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc) if (channel < 0) return 0;
- return vc4_crtc_disable(crtc, channel); + return vc4_crtc_disable(crtc, NULL, channel); }
static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, @@ -484,7 +486,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, /* Disable vblank irq handling before crtc is disabled. */ drm_crtc_vblank_off(crtc);
- vc4_crtc_disable(crtc, old_vc4_state->assigned_channel); + vc4_crtc_disable(crtc, state, old_vc4_state->assigned_channel);
/* * Make sure we issue a vblank event after disabling the CRTC if @@ -518,14 +520,14 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, vc4_hvs_atomic_enable(crtc, state);
if (vc4_encoder->pre_crtc_configure) - vc4_encoder->pre_crtc_configure(encoder); + vc4_encoder->pre_crtc_configure(encoder, state);
vc4_crtc_config_pv(crtc);
CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);
if (vc4_encoder->pre_crtc_enable) - vc4_encoder->pre_crtc_enable(encoder); + vc4_encoder->pre_crtc_enable(encoder, state);
/* When feeding the transposer block the pixelvalve is unneeded and * should not be enabled. @@ -534,7 +536,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
if (vc4_encoder->post_crtc_enable) - vc4_encoder->post_crtc_enable(encoder); + vc4_encoder->post_crtc_enable(encoder, state); }
static enum drm_mode_status vc4_crtc_mode_valid(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 5a115bc62cc8..051ad4e31e52 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -441,12 +441,12 @@ struct vc4_encoder { enum vc4_encoder_type type; u32 clock_select;
- void (*pre_crtc_configure)(struct drm_encoder *encoder); - void (*pre_crtc_enable)(struct drm_encoder *encoder); - void (*post_crtc_enable)(struct drm_encoder *encoder); + void (*pre_crtc_configure)(struct drm_encoder *encoder, struct drm_atomic_state *state); + void (*pre_crtc_enable)(struct drm_encoder *encoder, struct drm_atomic_state *state); + void (*post_crtc_enable)(struct drm_encoder *encoder, struct drm_atomic_state *state);
- void (*post_crtc_disable)(struct drm_encoder *encoder); - void (*post_crtc_powerdown)(struct drm_encoder *encoder); + void (*post_crtc_disable)(struct drm_encoder *encoder, struct drm_atomic_state *state); + void (*post_crtc_powerdown)(struct drm_encoder *encoder, struct drm_atomic_state *state); };
static inline struct vc4_encoder * diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 9f8d74e32355..8ce5dd65f6e4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -360,7 +360,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }
-static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder) +static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -373,7 +374,8 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder) HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX); }
-static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder) +static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); int ret; @@ -587,7 +589,8 @@ static void vc4_hdmi_recenter_fifo(struct vc4_hdmi *vc4_hdmi) "VC4_HDMI_FIFO_CTL_RECENTER_DONE"); }
-static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder) +static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); @@ -679,7 +682,8 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder) vc4_hdmi->variant->set_timings(vc4_hdmi, mode); }
-static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder) +static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder); @@ -701,7 +705,8 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder) HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N); }
-static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder) +static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
Commit 63495f6b4aed ("drm/vc4: hdmi: Make sure our clock rate is within limits") was intended to compute the pixel rate to make sure we remain within the boundaries of what the hardware can provide.
However, unlike what mode_valid was checking for, we forgot to take into account the clock doubling flag that can be set for modes. Let's honor that flag if it's there.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Reported-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Fixes: 63495f6b4aed ("drm/vc4: hdmi: Make sure our clock rate is within limits") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 8ce5dd65f6e4..3dac839b0fa5 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -799,6 +799,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, pixel_rate = mode->clock * 1000; }
+ if (mode->flags & DRM_MODE_FLAG_DBLCLK) + pixel_rate = pixel_rate * 2; + if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL;
drm_atomic_helper_connector_reset uses kmalloc which, from an API standpoint, can fail, and thus setting connector->state to NULL. However, our reset hook then calls drm_atomic_helper_connector_tv_reset that will access connector->state without checking if it's a valid pointer or not.
Make sure we don't end up accessing a NULL pointer.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Suggested-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 3dac839b0fa5..920895deb2e7 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -171,7 +171,9 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) static void vc4_hdmi_connector_reset(struct drm_connector *connector) { drm_atomic_helper_connector_reset(connector); - drm_atomic_helper_connector_tv_reset(connector); + + if (connector->state) + drm_atomic_helper_connector_tv_reset(connector); }
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
When run with a higher bpc than 8, the clock of the HDMI controller needs to be adjusted. Let's create a connector state that will be used at atomic_check and atomic_enable to compute and store the clock rate associated to the state.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 33 ++++++++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 10 ++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 920895deb2e7..d22a0dbd0ce2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -170,10 +170,37 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
static void vc4_hdmi_connector_reset(struct drm_connector *connector) { - drm_atomic_helper_connector_reset(connector); + struct vc4_hdmi_connector_state *old_state = + conn_state_to_vc4_hdmi_conn_state(connector->state); + struct vc4_hdmi_connector_state *new_state = + kzalloc(sizeof(*new_state), GFP_KERNEL);
if (connector->state) - drm_atomic_helper_connector_tv_reset(connector); + __drm_atomic_helper_connector_destroy_state(connector->state); + + kfree(old_state); + __drm_atomic_helper_connector_reset(connector, &new_state->base); + + if (!new_state) + return; + + drm_atomic_helper_connector_tv_reset(connector); +} + +static struct drm_connector_state * +vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) +{ + struct drm_connector_state *conn_state = connector->state; + struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state); + struct vc4_hdmi_connector_state *new_state; + + new_state = kzalloc(sizeof(*new_state), GFP_KERNEL); + if (!new_state) + return NULL; + + __drm_atomic_helper_connector_duplicate_state(connector, &new_state->base); + + return &new_state->base; }
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { @@ -181,7 +208,7 @@ static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vc4_hdmi_connector_destroy, .reset = vc4_hdmi_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 0526a9cf608a..2cf5362052e2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -180,6 +180,16 @@ encoder_to_vc4_hdmi(struct drm_encoder *encoder) return container_of(_encoder, struct vc4_hdmi, encoder); }
+struct vc4_hdmi_connector_state { + struct drm_connector_state base; +}; + +static inline struct vc4_hdmi_connector_state * +conn_state_to_vc4_hdmi_conn_state(struct drm_connector_state *conn_state) +{ + return container_of(conn_state, struct vc4_hdmi_connector_state, base); +} + void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode); void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi);
Hi Maxime
On Tue, 15 Dec 2020 at 15:42, Maxime Ripard maxime@cerno.tech wrote:
When run with a higher bpc than 8, the clock of the HDMI controller needs to be adjusted. Let's create a connector state that will be used at atomic_check and atomic_enable to compute and store the clock rate associated to the state.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Maxime Ripard maxime@cerno.tech
I'm happy again Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
drivers/gpu/drm/vc4/vc4_hdmi.c | 33 ++++++++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 10 ++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 920895deb2e7..d22a0dbd0ce2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -170,10 +170,37 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
static void vc4_hdmi_connector_reset(struct drm_connector *connector) {
drm_atomic_helper_connector_reset(connector);
struct vc4_hdmi_connector_state *old_state =conn_state_to_vc4_hdmi_conn_state(connector->state);struct vc4_hdmi_connector_state *new_state =kzalloc(sizeof(*new_state), GFP_KERNEL); if (connector->state)
drm_atomic_helper_connector_tv_reset(connector);
__drm_atomic_helper_connector_destroy_state(connector->state);kfree(old_state);__drm_atomic_helper_connector_reset(connector, &new_state->base);if (!new_state)return;drm_atomic_helper_connector_tv_reset(connector);+}
+static struct drm_connector_state * +vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) +{
struct drm_connector_state *conn_state = connector->state;struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state);struct vc4_hdmi_connector_state *new_state;new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);if (!new_state)return NULL;__drm_atomic_helper_connector_duplicate_state(connector, &new_state->base);return &new_state->base;}
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { @@ -181,7 +208,7 @@ static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vc4_hdmi_connector_destroy, .reset = vc4_hdmi_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,};
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 0526a9cf608a..2cf5362052e2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -180,6 +180,16 @@ encoder_to_vc4_hdmi(struct drm_encoder *encoder) return container_of(_encoder, struct vc4_hdmi, encoder); }
+struct vc4_hdmi_connector_state {
struct drm_connector_state base;+};
+static inline struct vc4_hdmi_connector_state * +conn_state_to_vc4_hdmi_conn_state(struct drm_connector_state *conn_state) +{
return container_of(conn_state, struct vc4_hdmi_connector_state, base);+}
void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode); void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi); -- 2.29.2
Hi Dave,
On Tue, Dec 15, 2020 at 04:25:46PM +0000, Dave Stevenson wrote:
Hi Maxime
On Tue, 15 Dec 2020 at 15:42, Maxime Ripard maxime@cerno.tech wrote:
When run with a higher bpc than 8, the clock of the HDMI controller needs to be adjusted. Let's create a connector state that will be used at atomic_check and atomic_enable to compute and store the clock rate associated to the state.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Signed-off-by: Maxime Ripard maxime@cerno.tech
I'm happy again Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
Thanks!
Does that apply to patch 9 as well?
Maxime
The pixel rate is for now quite simple to compute, but with more features (30 and 36 bits output, YUV output, etc.) will depend on a bunch of connectors properties.
Let's store the rate we have to run the pixel clock at in our custom connector state, and compute it in atomic_check.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 26 +++++++++++++++++++++++++- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + 2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index d22a0dbd0ce2..a6422d7af019 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -198,6 +198,7 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) if (!new_state) return NULL;
+ new_state->pixel_rate = vc4_state->pixel_rate; __drm_atomic_helper_connector_duplicate_state(connector, &new_state->base);
return &new_state->base; @@ -618,9 +619,29 @@ static void vc4_hdmi_recenter_fifo(struct vc4_hdmi *vc4_hdmi) "VC4_HDMI_FIFO_CTL_RECENTER_DONE"); }
+static struct drm_connector_state * +vc4_hdmi_encoder_get_connector_state(struct drm_encoder *encoder, + struct drm_atomic_state *state) +{ + struct drm_connector_state *conn_state; + struct drm_connector *connector; + unsigned int i; + + for_each_new_connector_in_state(state, connector, conn_state, i) { + if (conn_state->best_encoder == encoder) + return conn_state; + } + + return NULL; +} + static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, struct drm_atomic_state *state) { + struct drm_connector_state *conn_state = + vc4_hdmi_encoder_get_connector_state(encoder, state); + struct vc4_hdmi_connector_state *vc4_conn_state = + conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); unsigned long pixel_rate, hsm_rate; @@ -632,7 +653,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, return; }
- pixel_rate = mode->clock * 1000 * ((mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1); + pixel_rate = vc4_conn_state->pixel_rate; ret = clk_set_rate(vc4_hdmi->pixel_clock, pixel_rate); if (ret) { DRM_ERROR("Failed to set pixel clock rate: %d\n", ret); @@ -804,6 +825,7 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &crtc_state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); unsigned long long pixel_rate = mode->clock * 1000; @@ -834,6 +856,8 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL;
+ vc4_state->pixel_rate = pixel_rate; + return 0; }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 2cf5362052e2..bca6943de884 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -182,6 +182,7 @@ encoder_to_vc4_hdmi(struct drm_encoder *encoder)
struct vc4_hdmi_connector_state { struct drm_connector_state base; + unsigned long long pixel_rate; };
static inline struct vc4_hdmi_connector_state *
The PHY initialisation parameters are not based on the pixel clock but the TMDS clock rate which can be the pixel clock in the standard case, but could be adjusted based on some parameters like the bits per color.
Since the TMDS clock rate is stored in our custom connector state already, let's reuse it from there instead of computing it again.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +++++------ drivers/gpu/drm/vc4/vc4_hdmi_phy.c | 8 +++++--- 3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index a6422d7af019..dbe516d89726 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -721,7 +721,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, vc4_hdmi->variant->reset(vc4_hdmi);
if (vc4_hdmi->variant->phy_init) - vc4_hdmi->variant->phy_init(vc4_hdmi, mode); + vc4_hdmi->variant->phy_init(vc4_hdmi, vc4_conn_state);
HDMI_WRITE(HDMI_SCHEDULER_CONTROL, HDMI_READ(HDMI_SCHEDULER_CONTROL) | diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index bca6943de884..60c53d7c9bad 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -21,10 +21,9 @@ to_vc4_hdmi_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_hdmi_encoder, base.base); }
-struct drm_display_mode; - struct vc4_hdmi; struct vc4_hdmi_register; +struct vc4_hdmi_connector_state;
enum vc4_hdmi_phy_channel { PHY_LANE_0 = 0, @@ -80,9 +79,9 @@ struct vc4_hdmi_variant { void (*set_timings)(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode);
- /* Callback to initialize the PHY according to the mode */ + /* Callback to initialize the PHY according to the connector state */ void (*phy_init)(struct vc4_hdmi *vc4_hdmi, - struct drm_display_mode *mode); + struct vc4_hdmi_connector_state *vc4_conn_state);
/* Callback to disable the PHY */ void (*phy_disable)(struct vc4_hdmi *vc4_hdmi); @@ -192,13 +191,13 @@ conn_state_to_vc4_hdmi_conn_state(struct drm_connector_state *conn_state) }
void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, - struct drm_display_mode *mode); + struct vc4_hdmi_connector_state *vc4_conn_state); void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi); void vc4_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi); void vc4_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi);
void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, - struct drm_display_mode *mode); + struct vc4_hdmi_connector_state *vc4_conn_state); void vc5_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi); void vc5_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi); void vc5_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c index 057796b54c51..36535480f8e2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c @@ -127,7 +127,8 @@
#define OSCILLATOR_FREQUENCY 54000000
-void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode) +void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, + struct vc4_hdmi_connector_state *conn_state) { /* PHY should be in reset, like * vc4_hdmi_encoder_disable() does. @@ -339,11 +340,12 @@ static void vc5_hdmi_reset_phy(struct vc4_hdmi *vc4_hdmi) HDMI_WRITE(HDMI_TX_PHY_POWERDOWN_CTL, BIT(10)); }
-void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode) +void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, + struct vc4_hdmi_connector_state *conn_state) { const struct phy_lane_settings *chan0_settings, *chan1_settings, *chan2_settings, *clock_settings; const struct vc4_hdmi_variant *variant = vc4_hdmi->variant; - unsigned long long pixel_freq = mode->clock * 1000; + unsigned long long pixel_freq = conn_state->pixel_rate; unsigned long long vco_freq; unsigned char word_sel; u8 vco_sel, vco_div;
Unlike the previous generations, the HSM clock limitation is way above what we can reach without scrambling, so let's move the maximum frequency we support to the maximum clock frequency without scrambling.
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index dbe516d89726..41897b8e9d51 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -82,6 +82,8 @@ #define CEC_CLOCK_FREQ 40000 #define VC4_HSM_MID_CLOCK 149985000
+#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000) + static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *)m->private; @@ -1918,7 +1920,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = { .encoder_type = VC4_ENCODER_TYPE_HDMI0, .debugfs_name = "hdmi0_regs", .card_name = "vc4-hdmi-0", - .max_pixel_clock = 297000000, + .max_pixel_clock = HDMI_14_MAX_TMDS_CLK, .registers = vc5_hdmi_hdmi0_fields, .num_registers = ARRAY_SIZE(vc5_hdmi_hdmi0_fields), .phy_lane_mapping = { @@ -1944,7 +1946,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = { .encoder_type = VC4_ENCODER_TYPE_HDMI1, .debugfs_name = "hdmi1_regs", .card_name = "vc4-hdmi-1", - .max_pixel_clock = 297000000, + .max_pixel_clock = HDMI_14_MAX_TMDS_CLK, .registers = vc5_hdmi_hdmi1_fields, .num_registers = ARRAY_SIZE(vc5_hdmi_hdmi1_fields), .phy_lane_mapping = {
The BCM2711 supports higher bpc count than just 8, so let's support it in our driver.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 70 ++++++++++++++++++++++++++++- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 9 ++++ 3 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 41897b8e9d51..2e5449b25ce4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -76,6 +76,17 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 +#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8) + +#define VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_SHIFT 0 +#define VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_MASK VC4_MASK(3, 0) + +#define VC5_HDMI_GCP_CONFIG_GCP_ENABLE BIT(31) + +#define VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_SHIFT 8 +#define VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_MASK VC4_MASK(15, 8) + # define VC4_HD_M_SW_RST BIT(2) # define VC4_HD_M_ENABLE BIT(0)
@@ -186,6 +197,8 @@ static void vc4_hdmi_connector_reset(struct drm_connector *connector) if (!new_state) return;
+ new_state->base.max_bpc = 8; + new_state->base.max_requested_bpc = 8; drm_atomic_helper_connector_tv_reset(connector); }
@@ -232,12 +245,20 @@ static int vc4_hdmi_connector_init(struct drm_device *dev, vc4_hdmi->ddc); drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
+ /* + * Some of the properties below require access to state, like bpc. + * Allocate some default initial connector state with our reset helper. + */ + if (connector->funcs->reset) + connector->funcs->reset(connector); + /* Create and attach TV margin props to this connector. */ ret = drm_mode_create_tv_margin_properties(dev); if (ret) return ret;
drm_connector_attach_tv_margin_properties(connector); + drm_connector_attach_max_bpc_property(connector, 8, 12);
connector->polled = (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT); @@ -506,6 +527,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, bool enable) }
static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, + struct drm_connector_state *state, struct drm_display_mode *mode) { bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -549,7 +571,9 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_VERTB0, vertb_even); HDMI_WRITE(HDMI_VERTB1, vertb); } + static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, + struct drm_connector_state *state, struct drm_display_mode *mode) { bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -569,6 +593,9 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, mode->crtc_vsync_end - interlaced, VC4_HDMI_VERTB_VBP)); + unsigned char gcp; + bool gcp_en; + u32 reg;
HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021); HDMI_WRITE(HDMI_HORZA, @@ -594,6 +621,39 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_VERTB0, vertb_even); HDMI_WRITE(HDMI_VERTB1, vertb);
+ switch (state->max_bpc) { + case 12: + gcp = 6; + gcp_en = true; + break; + case 10: + gcp = 5; + gcp_en = true; + break; + case 8: + default: + gcp = 4; + gcp_en = false; + break; + } + + reg = HDMI_READ(HDMI_DEEP_COLOR_CONFIG_1); + reg &= ~(VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK | + VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_MASK); + reg |= VC4_SET_FIELD(2, VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE) | + VC4_SET_FIELD(gcp, VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH); + HDMI_WRITE(HDMI_DEEP_COLOR_CONFIG_1, reg); + + reg = HDMI_READ(HDMI_GCP_WORD_1); + reg &= ~VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_MASK; + reg |= VC4_SET_FIELD(gcp, VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1); + HDMI_WRITE(HDMI_GCP_WORD_1, reg); + + reg = HDMI_READ(HDMI_GCP_CONFIG); + reg &= ~VC5_HDMI_GCP_CONFIG_GCP_ENABLE; + reg |= gcp_en ? VC5_HDMI_GCP_CONFIG_GCP_ENABLE : 0; + HDMI_WRITE(HDMI_GCP_CONFIG, reg); + HDMI_WRITE(HDMI_CLOCK_STOP, 0); }
@@ -731,7 +791,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, VC4_HDMI_SCHEDULER_CONTROL_IGNORE_VSYNC_PREDICTS);
if (vc4_hdmi->variant->set_timings) - vc4_hdmi->variant->set_timings(vc4_hdmi, mode); + vc4_hdmi->variant->set_timings(vc4_hdmi, conn_state, mode); }
static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, @@ -852,6 +912,14 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, pixel_rate = mode->clock * 1000; }
+ if (conn_state->max_bpc == 12) { + pixel_rate = pixel_rate * 150; + do_div(pixel_rate, 100); + } else if (conn_state->max_bpc == 10) { + pixel_rate = pixel_rate * 125; + do_div(pixel_rate, 100); + } + if (mode->flags & DRM_MODE_FLAG_DBLCLK) pixel_rate = pixel_rate * 2;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 60c53d7c9bad..4c8994cfd932 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -77,6 +77,7 @@ struct vc4_hdmi_variant {
/* Callback to configure the video timings in the HDMI block */ void (*set_timings)(struct vc4_hdmi *vc4_hdmi, + struct drm_connector_state *state, struct drm_display_mode *mode);
/* Callback to initialize the PHY according to the connector state */ diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 96d764ebfe67..401863cb8c98 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -59,9 +59,12 @@ enum vc4_hdmi_field { */ HDMI_CTS_0, HDMI_CTS_1, + HDMI_DEEP_COLOR_CONFIG_1, HDMI_DVP_CTL, HDMI_FIFO_CTL, HDMI_FRAME_COUNT, + HDMI_GCP_CONFIG, + HDMI_GCP_WORD_1, HDMI_HORZA, HDMI_HORZB, HDMI_HOTPLUG, @@ -229,6 +232,9 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), + VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170), + VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), + VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),
VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc), @@ -305,6 +311,9 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), + VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170), + VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), + VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),
VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),
Hi Maxime
On Tue, 15 Dec 2020 at 15:43, Maxime Ripard maxime@cerno.tech wrote:
The BCM2711 supports higher bpc count than just 8, so let's support it in our driver.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
drivers/gpu/drm/vc4/vc4_hdmi.c | 70 ++++++++++++++++++++++++++++- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 9 ++++ 3 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 41897b8e9d51..2e5449b25ce4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -76,6 +76,17 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 +#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
+#define VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_SHIFT 0 +#define VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_MASK VC4_MASK(3, 0)
+#define VC5_HDMI_GCP_CONFIG_GCP_ENABLE BIT(31)
+#define VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_SHIFT 8 +#define VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_MASK VC4_MASK(15, 8)
# define VC4_HD_M_SW_RST BIT(2) # define VC4_HD_M_ENABLE BIT(0)
@@ -186,6 +197,8 @@ static void vc4_hdmi_connector_reset(struct drm_connector *connector) if (!new_state) return;
new_state->base.max_bpc = 8;new_state->base.max_requested_bpc = 8; drm_atomic_helper_connector_tv_reset(connector);}
@@ -232,12 +245,20 @@ static int vc4_hdmi_connector_init(struct drm_device *dev, vc4_hdmi->ddc); drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
/** Some of the properties below require access to state, like bpc.* Allocate some default initial connector state with our reset helper.*/if (connector->funcs->reset)connector->funcs->reset(connector);/* Create and attach TV margin props to this connector. */ ret = drm_mode_create_tv_margin_properties(dev); if (ret) return ret; drm_connector_attach_tv_margin_properties(connector);drm_connector_attach_max_bpc_property(connector, 8, 12); connector->polled = (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT);@@ -506,6 +527,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, bool enable) }
static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state, struct drm_display_mode *mode){ bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -549,7 +571,9 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_VERTB0, vertb_even); HDMI_WRITE(HDMI_VERTB1, vertb); }
static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state, struct drm_display_mode *mode){ bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -569,6 +593,9 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, mode->crtc_vsync_end - interlaced, VC4_HDMI_VERTB_VBP));
unsigned char gcp;bool gcp_en;u32 reg; HDMI_WRITE(HDMI_VEC_INTERFACE_XBAR, 0x354021); HDMI_WRITE(HDMI_HORZA,@@ -594,6 +621,39 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_VERTB0, vertb_even); HDMI_WRITE(HDMI_VERTB1, vertb);
switch (state->max_bpc) {case 12:gcp = 6;gcp_en = true;break;case 10:gcp = 5;gcp_en = true;break;case 8:default:gcp = 4;gcp_en = false;break;}reg = HDMI_READ(HDMI_DEEP_COLOR_CONFIG_1);reg &= ~(VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK |VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH_MASK);reg |= VC4_SET_FIELD(2, VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE) |VC4_SET_FIELD(gcp, VC5_HDMI_DEEP_COLOR_CONFIG_1_COLOR_DEPTH);HDMI_WRITE(HDMI_DEEP_COLOR_CONFIG_1, reg);reg = HDMI_READ(HDMI_GCP_WORD_1);reg &= ~VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1_MASK;reg |= VC4_SET_FIELD(gcp, VC5_HDMI_GCP_WORD_1_GCP_SUBPACKET_BYTE_1);HDMI_WRITE(HDMI_GCP_WORD_1, reg);reg = HDMI_READ(HDMI_GCP_CONFIG);reg &= ~VC5_HDMI_GCP_CONFIG_GCP_ENABLE;reg |= gcp_en ? VC5_HDMI_GCP_CONFIG_GCP_ENABLE : 0;HDMI_WRITE(HDMI_GCP_CONFIG, reg);HDMI_WRITE(HDMI_CLOCK_STOP, 0);}
@@ -731,7 +791,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, VC4_HDMI_SCHEDULER_CONTROL_IGNORE_VSYNC_PREDICTS);
if (vc4_hdmi->variant->set_timings)
vc4_hdmi->variant->set_timings(vc4_hdmi, mode);
vc4_hdmi->variant->set_timings(vc4_hdmi, conn_state, mode);}
static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, @@ -852,6 +912,14 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, pixel_rate = mode->clock * 1000; }
if (conn_state->max_bpc == 12) {pixel_rate = pixel_rate * 150;do_div(pixel_rate, 100);} else if (conn_state->max_bpc == 10) {pixel_rate = pixel_rate * 125;do_div(pixel_rate, 100);}if (mode->flags & DRM_MODE_FLAG_DBLCLK) pixel_rate = pixel_rate * 2;diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 60c53d7c9bad..4c8994cfd932 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -77,6 +77,7 @@ struct vc4_hdmi_variant {
/* Callback to configure the video timings in the HDMI block */ void (*set_timings)(struct vc4_hdmi *vc4_hdmi,
struct drm_connector_state *state, struct drm_display_mode *mode); /* Callback to initialize the PHY according to the connector state */diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 96d764ebfe67..401863cb8c98 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -59,9 +59,12 @@ enum vc4_hdmi_field { */ HDMI_CTS_0, HDMI_CTS_1,
HDMI_DEEP_COLOR_CONFIG_1, HDMI_DVP_CTL, HDMI_FIFO_CTL, HDMI_FRAME_COUNT,HDMI_GCP_CONFIG,HDMI_GCP_WORD_1, HDMI_HORZA, HDMI_HORZB, HDMI_HOTPLUG,@@ -229,6 +232,9 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0),
VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170),VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178),VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8), VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),@@ -305,6 +311,9 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0),
VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170),VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178),VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8), VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),-- 2.29.2
dri-devel@lists.freedesktop.org
-
Dave Stevenson -
Maxime Ripard