Hi,
Here's a collection of small fixes that have been used in the downstream kernel for a while, affecting several parts of the vc4 driver.
Let me know what you think, Maxime
Dave Stevenson (21): drm/vc4: drv: Adopt the dma configuration from the HVS or V3D component drm/vc4: plane: Fix margin calculations for the right/bottom edges drm/vc4: plane: Add alpha_blend_mode property to each plane. drm/vc4: hvs: Add debugfs node that dumps the current display lists drm/vc4: dpi: Add support for composite syncs to vc4_dpi drm/vc4: dpi: Add option for inverting pixel clock and output enable drm/vc4: dpi: Ensure a default format is selected drm/vc4: dsi: Release workaround buffer and DMA drm/vc4: dsi: Correct DSI divider calculations drm/vc4: dsi: Correct pixel order for DSI0 drm/vc4: dsi: Register dsi0 as the correct vc4 encoder type drm/vc4: dsi: Fix dsi0 interrupt support drm/vc4: dsi: Add correct stop condition to vc4_dsi_encoder_disable iteration drm/vc4: hdmi: Add all the vc5 HDMI registers into the debugfs dumps drm/vc4: hdmi: Reset HDMI MISC_CONTROL register drm/vc4: hdmi: Switch to pm_runtime_status_suspended drm/vc4: hdmi: Move HDMI reset to pm_resume drm/vc4: hdmi: Add HDMI format detection registers to register list drm/vc4: hdmi: Add MISC_CONTROL register for vc4 drm/vc4: hdmi: Correct HDMI timing registers for interlaced modes drm/vc4: hdmi: Move pixel doubling from Pixelvalve to HDMI block
Dom Cobley (8): drm/vc4: plane: Remove subpixel positioning check drm/vc4: hdmi: Clear unused infoframe packet RAM registers drm/vc4: hdmi: Avoid full hdmi audio fifo writes drm/vc4: hdmi: Stop checking for enabled output in audio drm/vc4: hdmi: Skip writes to disabled packet RAM drm/vc4: hdmi: Remove VC4_HDMI_SCHEDULER_CONTROL_VERT_ALWAYS_KEEPOUT drm/vc4: hdmi: Report that 3d/stereo is allowed drm/vc4: hdmi: Force modeset when bpc or format changes
Mateusz Kwiatkowski (1): drm/vc4: hdmi: Fix timings for interlaced modes
Maxime Ripard (2): drm/vc4: kms: Warn if clk_set_min_rate fails drm/vc4: kms: Use maximum FIFO load for the HVS clock rate
Phil Elwell (1): drm/vc4: hdmi: Disable audio if dmas property is present but empty
drivers/gpu/drm/vc4/vc4_crtc.c | 14 +- drivers/gpu/drm/vc4/vc4_dpi.c | 99 +++++++------ drivers/gpu/drm/vc4/vc4_drv.c | 19 +++ drivers/gpu/drm/vc4/vc4_dsi.c | 152 +++++++++++++++----- drivers/gpu/drm/vc4/vc4_hdmi.c | 210 ++++++++++++++++++---------- drivers/gpu/drm/vc4/vc4_hdmi.h | 14 +- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 38 ++++- drivers/gpu/drm/vc4/vc4_hvs.c | 42 ++++++ drivers/gpu/drm/vc4/vc4_kms.c | 8 +- drivers/gpu/drm/vc4/vc4_plane.c | 92 ++++++++---- 10 files changed, 503 insertions(+), 185 deletions(-)
From: Dave Stevenson dave.stevenson@raspberrypi.com
vc4_drv isn't necessarily under the /soc node in DT as it is a virtual device, but it is the one that does the allocations. The DMA addresses are consumed by primarily the HVS or V3D, and those require VideoCore cache alias address mapping, and so will be under /soc.
During probe find the a suitable device node for HVS or V3D, and adopt the DMA configuration of that node.
Cc: stable@vger.kernel.org Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_drv.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 162bc18e7497..14a7d529144d 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -209,6 +209,15 @@ static void vc4_match_add_drivers(struct device *dev, } }
+const struct of_device_id vc4_dma_range_matches[] = { + { .compatible = "brcm,bcm2711-hvs" }, + { .compatible = "brcm,bcm2835-hvs" }, + { .compatible = "brcm,bcm2835-v3d" }, + { .compatible = "brcm,cygnus-v3d" }, + { .compatible = "brcm,vc4-v3d" }, + {} +}; + static int vc4_drm_bind(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); @@ -227,6 +236,16 @@ static int vc4_drm_bind(struct device *dev) vc4_drm_driver.driver_features &= ~DRIVER_RENDER; of_node_put(node);
+ node = of_find_matching_node_and_match(NULL, vc4_dma_range_matches, + NULL); + if (node) { + ret = of_dma_configure(dev, node, true); + of_node_put(node); + + if (ret) + return ret; + } + vc4 = devm_drm_dev_alloc(dev, &vc4_drm_driver, struct vc4_dev, base); if (IS_ERR(vc4)) return PTR_ERR(vc4);
We currently ignore the clk_set_min_rate return code assuming it would succeed. However, it can fail if we ask for a rate higher than the current maximum for example.
Since we can't fail in atomic_commit, at least warn on failure.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index c169bd72e53b..7a7c90d8520b 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -405,7 +405,7 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state) * Do a temporary request on the core clock during the * modeset. */ - clk_set_min_rate(hvs->core_clk, core_rate); + WARN_ON(clk_set_min_rate(hvs->core_clk, core_rate)); }
drm_atomic_helper_commit_modeset_disables(dev, state); @@ -438,7 +438,7 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state) * Request a clock rate based on the current HVS * requirements. */ - clk_set_min_rate(hvs->core_clk, new_hvs_state->core_clock_rate); + WARN_ON(clk_set_min_rate(hvs->core_clk, new_hvs_state->core_clock_rate));
drm_dbg(dev, "Core clock actual rate: %lu Hz\n", clk_get_rate(hvs->core_clk));
The core clock computation takes into account both the load due to the input (ie, planes) and its output (ie, encoders).
However, while the input load needs to consider all the planes, and thus sum all of their associated loads, the output happens mostly in parallel.
Therefore, we need to consider only the maximum of all the output loads, and not the sum like we were doing. This resulted in a clock rate way too high which could be discarded for being too high by the clock framework.
Since recent changes, the clock framework will even downright reject it, leading to a core clock being too low for its current needs.
Fixes: 16e101051f32 ("drm/vc4: Increase the core clock based on HVS load") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_kms.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 7a7c90d8520b..69eae37e82f6 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -946,7 +946,9 @@ vc4_core_clock_atomic_check(struct drm_atomic_state *state) continue;
num_outputs++; - cob_rate += hvs_new_state->fifo_state[i].fifo_load; + cob_rate = max_t(unsigned long, + hvs_new_state->fifo_state[i].fifo_load, + cob_rate); }
pixel_rate = load_state->hvs_load;
From: Dom Cobley popcornmix@gmail.com
There is little harm in ignoring fractional coordinates (they just get truncated).
Without this: modetest -M vc4 -F tiles,gradient -s 32:1920x1080-60 -P89@74:1920x1080*.1.1@XR24
is rejected. We have the same issue in Kodi when trying to use zoom options on video.
Note: even if all coordinates are fully integer. e.g. src:[0,0,1920,1080] dest:[-10,-10,1940,1100]
it will still get rejected as drm_atomic_helper_check_plane_state uses drm_rect_clip_scaled which transforms this to fractional src coords
Fixes: 21af94cf1a4c ("drm/vc4: Add support for scaling of display planes.") Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_plane.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index b3438f4a81ce..650c652281e8 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -339,7 +339,6 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); struct drm_framebuffer *fb = state->fb; struct drm_gem_cma_object *bo = drm_fb_cma_get_gem_obj(fb, 0); - u32 subpixel_src_mask = (1 << 16) - 1; int num_planes = fb->format->num_planes; struct drm_crtc_state *crtc_state; u32 h_subsample = fb->format->hsub; @@ -361,18 +360,15 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) for (i = 0; i < num_planes; i++) vc4_state->offsets[i] = bo->paddr + fb->offsets[i];
- /* We don't support subpixel source positioning for scaling. */ - if ((state->src.x1 & subpixel_src_mask) || - (state->src.x2 & subpixel_src_mask) || - (state->src.y1 & subpixel_src_mask) || - (state->src.y2 & subpixel_src_mask)) { - return -EINVAL; - } - - vc4_state->src_x = state->src.x1 >> 16; - vc4_state->src_y = state->src.y1 >> 16; - vc4_state->src_w[0] = (state->src.x2 - state->src.x1) >> 16; - vc4_state->src_h[0] = (state->src.y2 - state->src.y1) >> 16; + /* + * We don't support subpixel source positioning for scaling, + * but fractional coordinates can be generated by clipping + * so just round for now + */ + vc4_state->src_x = DIV_ROUND_CLOSEST(state->src.x1, 1 << 16); + vc4_state->src_y = DIV_ROUND_CLOSEST(state->src.y1, 1 << 16); + vc4_state->src_w[0] = DIV_ROUND_CLOSEST(state->src.x2, 1 << 16) - vc4_state->src_x; + vc4_state->src_h[0] = DIV_ROUND_CLOSEST(state->src.y2, 1 << 16) - vc4_state->src_y;
vc4_state->crtc_x = state->dst.x1; vc4_state->crtc_y = state->dst.y1;
From: Dave Stevenson dave.stevenson@raspberrypi.com
The current plane margin calculation code clips the right and bottom edges of the range based using the left and top margins.
This is obviously wrong, so let's fix it.
Fixes: 666e73587f90 ("drm/vc4: Take margin setup into account when updating planes") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_plane.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 650c652281e8..a64324179650 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -310,16 +310,16 @@ static int vc4_plane_margins_adj(struct drm_plane_state *pstate) adjhdisplay, crtc_state->mode.hdisplay); vc4_pstate->crtc_x += left; - if (vc4_pstate->crtc_x > crtc_state->mode.hdisplay - left) - vc4_pstate->crtc_x = crtc_state->mode.hdisplay - left; + if (vc4_pstate->crtc_x > crtc_state->mode.hdisplay - right) + vc4_pstate->crtc_x = crtc_state->mode.hdisplay - right;
adjvdisplay = crtc_state->mode.vdisplay - (top + bottom); vc4_pstate->crtc_y = DIV_ROUND_CLOSEST(vc4_pstate->crtc_y * adjvdisplay, crtc_state->mode.vdisplay); vc4_pstate->crtc_y += top; - if (vc4_pstate->crtc_y > crtc_state->mode.vdisplay - top) - vc4_pstate->crtc_y = crtc_state->mode.vdisplay - top; + if (vc4_pstate->crtc_y > crtc_state->mode.vdisplay - bottom) + vc4_pstate->crtc_y = crtc_state->mode.vdisplay - bottom;
vc4_pstate->crtc_w = DIV_ROUND_CLOSEST(vc4_pstate->crtc_w * adjhdisplay,
From: Dave Stevenson dave.stevenson@raspberrypi.com
Move from only supporting the default of pre-multiplied alpha to supporting user specified blend mode using the standardised property.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_plane.c | 62 ++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index a64324179650..ac250a592fad 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -664,6 +664,48 @@ static const u32 colorspace_coeffs[2][DRM_COLOR_ENCODING_MAX][3] = { } };
+static u32 vc4_hvs4_get_alpha_blend_mode(struct drm_plane_state *state) +{ + if (!state->fb->format->has_alpha) + return VC4_SET_FIELD(SCALER_POS2_ALPHA_MODE_FIXED, + SCALER_POS2_ALPHA_MODE); + + switch (state->pixel_blend_mode) { + case DRM_MODE_BLEND_PIXEL_NONE: + return VC4_SET_FIELD(SCALER_POS2_ALPHA_MODE_FIXED, + SCALER_POS2_ALPHA_MODE); + default: + case DRM_MODE_BLEND_PREMULTI: + return VC4_SET_FIELD(SCALER_POS2_ALPHA_MODE_PIPELINE, + SCALER_POS2_ALPHA_MODE) | + SCALER_POS2_ALPHA_PREMULT; + case DRM_MODE_BLEND_COVERAGE: + return VC4_SET_FIELD(SCALER_POS2_ALPHA_MODE_PIPELINE, + SCALER_POS2_ALPHA_MODE); + } +} + +static u32 vc4_hvs5_get_alpha_blend_mode(struct drm_plane_state *state) +{ + if (!state->fb->format->has_alpha) + return VC4_SET_FIELD(SCALER5_CTL2_ALPHA_MODE_FIXED, + SCALER5_CTL2_ALPHA_MODE); + + switch (state->pixel_blend_mode) { + case DRM_MODE_BLEND_PIXEL_NONE: + return VC4_SET_FIELD(SCALER5_CTL2_ALPHA_MODE_FIXED, + SCALER5_CTL2_ALPHA_MODE); + default: + case DRM_MODE_BLEND_PREMULTI: + return VC4_SET_FIELD(SCALER5_CTL2_ALPHA_MODE_PIPELINE, + SCALER5_CTL2_ALPHA_MODE) | + SCALER5_CTL2_ALPHA_PREMULT; + case DRM_MODE_BLEND_COVERAGE: + return VC4_SET_FIELD(SCALER5_CTL2_ALPHA_MODE_PIPELINE, + SCALER5_CTL2_ALPHA_MODE); + } +} + /* Writes out a full display list for an active plane to the plane's * private dlist state. */ @@ -946,13 +988,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, /* Position Word 2: Source Image Size, Alpha */ vc4_state->pos2_offset = vc4_state->dlist_count; vc4_dlist_write(vc4_state, - VC4_SET_FIELD(fb->format->has_alpha ? - SCALER_POS2_ALPHA_MODE_PIPELINE : - SCALER_POS2_ALPHA_MODE_FIXED, - SCALER_POS2_ALPHA_MODE) | (mix_plane_alpha ? SCALER_POS2_ALPHA_MIX : 0) | - (fb->format->has_alpha ? - SCALER_POS2_ALPHA_PREMULT : 0) | + vc4_hvs4_get_alpha_blend_mode(state) | VC4_SET_FIELD(vc4_state->src_w[0], SCALER_POS2_WIDTH) | VC4_SET_FIELD(vc4_state->src_h[0], @@ -997,14 +1034,9 @@ static int vc4_plane_mode_set(struct drm_plane *plane, vc4_dlist_write(vc4_state, VC4_SET_FIELD(state->alpha >> 4, SCALER5_CTL2_ALPHA) | - (fb->format->has_alpha ? - SCALER5_CTL2_ALPHA_PREMULT : 0) | + vc4_hvs5_get_alpha_blend_mode(state) | (mix_plane_alpha ? - SCALER5_CTL2_ALPHA_MIX : 0) | - VC4_SET_FIELD(fb->format->has_alpha ? - SCALER5_CTL2_ALPHA_MODE_PIPELINE : - SCALER5_CTL2_ALPHA_MODE_FIXED, - SCALER5_CTL2_ALPHA_MODE) + SCALER5_CTL2_ALPHA_MIX : 0) );
/* Position Word 1: Scaled Image Dimensions. */ @@ -1489,6 +1521,10 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev, drm_plane_helper_add(plane, &vc4_plane_helper_funcs);
drm_plane_create_alpha_property(plane); + drm_plane_create_blend_mode_property(plane, + BIT(DRM_MODE_BLEND_PIXEL_NONE) | + BIT(DRM_MODE_BLEND_PREMULTI) | + BIT(DRM_MODE_BLEND_COVERAGE)); drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0, DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 |
From: Dave Stevenson dave.stevenson@raspberrypi.com
This allows easy analysis of display lists when debugging.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hvs.c | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 2a58fc421cf6..5fdfc01fbc5a 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -94,6 +94,46 @@ static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data) return 0; }
+static int vc4_hvs_debugfs_dlist(struct seq_file *m, void *data) +{ + struct drm_info_node *node = m->private; + struct drm_device *dev = node->minor->dev; + struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_hvs *hvs = vc4->hvs; + struct drm_printer p = drm_seq_file_printer(m); + unsigned int next_entry_start = 0; + unsigned int i, j; + u32 dlist_word, dispstat; + + for (i = 0; i < SCALER_CHANNELS_COUNT; i++) { + dispstat = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTATX(i)), + SCALER_DISPSTATX_MODE); + if (dispstat == SCALER_DISPSTATX_MODE_DISABLED || + dispstat == SCALER_DISPSTATX_MODE_EOF) { + drm_printf(&p, "HVS chan %u disabled\n", i); + continue; + } + + drm_printf(&p, "HVS chan %u:\n", i); + + for (j = HVS_READ(SCALER_DISPLISTX(i)); j < 256; j++) { + dlist_word = readl((u32 __iomem *)vc4->hvs->dlist + j); + drm_printf(&p, "dlist: %02d: 0x%08x\n", j, + dlist_word); + if (!next_entry_start || + next_entry_start == j) { + if (dlist_word & SCALER_CTL0_END) + break; + next_entry_start = j + + VC4_GET_FIELD(dlist_word, + SCALER_CTL0_SIZE); + } + } + } + + return 0; +} + /* The filter kernel is composed of dwords each containing 3 9-bit * signed integers packed next to each other. */ @@ -734,6 +774,8 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data) vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset); vc4_debugfs_add_file(drm, "hvs_underrun", vc4_hvs_debugfs_underrun, NULL); + vc4_debugfs_add_file(drm, "hvs_dlists", vc4_hvs_debugfs_dlist, + NULL);
return 0; }
From: Dave Stevenson dave.stevenson@raspberrypi.com
The hardware can combine H&V syncs onto the output enable line as composite syncs, so add the relevant configuration to do that.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dpi.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index c180eb60bee8..ffa55952c773 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -131,7 +131,7 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) struct vc4_dpi *dpi = vc4_encoder->dpi; struct drm_connector_list_iter conn_iter; struct drm_connector *connector = NULL, *connector_scan; - u32 dpi_c = DPI_ENABLE | DPI_OUTPUT_ENABLE_MODE; + u32 dpi_c = DPI_ENABLE; int ret;
/* Look up the connector attached to DPI so we can get the @@ -182,15 +182,22 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT); }
- if (mode->flags & DRM_MODE_FLAG_NHSYNC) - dpi_c |= DPI_HSYNC_INVERT; - else if (!(mode->flags & DRM_MODE_FLAG_PHSYNC)) - dpi_c |= DPI_HSYNC_DISABLE; + if (mode->flags & DRM_MODE_FLAG_CSYNC) { + if (mode->flags & DRM_MODE_FLAG_NCSYNC) + dpi_c |= DPI_OUTPUT_ENABLE_INVERT; + } else { + dpi_c |= DPI_OUTPUT_ENABLE_MODE;
- if (mode->flags & DRM_MODE_FLAG_NVSYNC) - dpi_c |= DPI_VSYNC_INVERT; - else if (!(mode->flags & DRM_MODE_FLAG_PVSYNC)) - dpi_c |= DPI_VSYNC_DISABLE; + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + dpi_c |= DPI_HSYNC_INVERT; + else if (!(mode->flags & DRM_MODE_FLAG_PHSYNC)) + dpi_c |= DPI_HSYNC_DISABLE; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + dpi_c |= DPI_VSYNC_INVERT; + else if (!(mode->flags & DRM_MODE_FLAG_PVSYNC)) + dpi_c |= DPI_VSYNC_DISABLE; + }
DPI_WRITE(DPI_C, dpi_c);
From: Dave Stevenson dave.stevenson@raspberrypi.com
DRM provides flags for inverting pixel clock and output enable signals, but these were not mapped to the relevant registers.
Add those mappings.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dpi.c | 64 ++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index ffa55952c773..695b759db9bc 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -148,35 +148,45 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) } drm_connector_list_iter_end(&conn_iter);
- if (connector && connector->display_info.num_bus_formats) { - u32 bus_format = connector->display_info.bus_formats[0]; + if (connector) { + if (connector->display_info.num_bus_formats) { + u32 bus_format = connector->display_info.bus_formats[0];
- switch (bus_format) { - case MEDIA_BUS_FMT_RGB888_1X24: - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, - DPI_FORMAT); - break; - case MEDIA_BUS_FMT_BGR888_1X24: - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, - DPI_FORMAT); - dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, DPI_ORDER); - break; - case MEDIA_BUS_FMT_RGB666_1X24_CPADHI: - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_2, - DPI_FORMAT); - break; - case MEDIA_BUS_FMT_RGB666_1X18: - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1, - DPI_FORMAT); - break; - case MEDIA_BUS_FMT_RGB565_1X16: - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3, - DPI_FORMAT); - break; - default: - DRM_ERROR("Unknown media bus format %d\n", bus_format); - break; + switch (bus_format) { + case MEDIA_BUS_FMT_RGB888_1X24: + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, + DPI_FORMAT); + break; + case MEDIA_BUS_FMT_BGR888_1X24: + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, + DPI_FORMAT); + dpi_c |= VC4_SET_FIELD(DPI_ORDER_BGR, + DPI_ORDER); + break; + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI: + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_2, + DPI_FORMAT); + break; + case MEDIA_BUS_FMT_RGB666_1X18: + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_18BIT_666_RGB_1, + DPI_FORMAT); + break; + case MEDIA_BUS_FMT_RGB565_1X16: + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_16BIT_565_RGB_3, + DPI_FORMAT); + break; + default: + DRM_ERROR("Unknown media bus format %d\n", + bus_format); + break; + } } + + if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE) + dpi_c |= DPI_PIXEL_CLK_INVERT; + + if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW) + dpi_c |= DPI_OUTPUT_ENABLE_INVERT; } else { /* Default to 24bit if no connector found. */ dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT);
From: Dave Stevenson dave.stevenson@raspberrypi.com
In a couple of error/incomplete configuration cases, the DPI_FORMAT bits wouldn't get set.
Enforce our RGB888 default in all these cases.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dpi.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 695b759db9bc..44355b347ff2 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -148,10 +148,15 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) } drm_connector_list_iter_end(&conn_iter);
+ /* Default to 24bit if no connector or format found. */ + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT); + if (connector) { if (connector->display_info.num_bus_formats) { u32 bus_format = connector->display_info.bus_formats[0];
+ dpi_c &= ~DPI_FORMAT_MASK; + switch (bus_format) { case MEDIA_BUS_FMT_RGB888_1X24: dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, @@ -187,9 +192,6 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder)
if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW) dpi_c |= DPI_OUTPUT_ENABLE_INVERT; - } else { - /* Default to 24bit if no connector found. */ - dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT); }
if (mode->flags & DRM_MODE_FLAG_CSYNC) {
From: Dave Stevenson dave.stevenson@raspberrypi.com
On Pi0-3 the driver allocates a buffer and requests a DMA channel because the ARM can't write to DSI1's registers directly.
However, we never release that buffer or channel. Let's add a device-managed action to release each.
Fixes: 4078f5757144 ("drm/vc4: Add DSI driver") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 98308a17e4ed..e82ee94cafc7 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1487,13 +1487,29 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) dsi->clk_onecell); }
+static void vc4_dsi_dma_mem_release(void *ptr) +{ + struct vc4_dsi *dsi = ptr; + struct device *dev = &dsi->pdev->dev; + + dma_free_coherent(dev, 4, dsi->reg_dma_mem, dsi->reg_dma_paddr); + dsi->reg_dma_mem = NULL; +} + +static void vc4_dsi_dma_chan_release(void *ptr) +{ + struct vc4_dsi *dsi = ptr; + + dma_release_channel(dsi->reg_dma_chan); + dsi->reg_dma_chan = NULL; +} + static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = dev_get_drvdata(master); struct vc4_dsi *dsi = dev_get_drvdata(dev); struct vc4_dsi_encoder *vc4_dsi_encoder; - dma_cap_mask_t dma_mask; int ret;
dsi->variant = of_device_get_match_data(dev); @@ -1527,6 +1543,8 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) * so set up a channel for talking to it. */ if (dsi->variant->broken_axi_workaround) { + dma_cap_mask_t dma_mask; + dsi->reg_dma_mem = dma_alloc_coherent(dev, 4, &dsi->reg_dma_paddr, GFP_KERNEL); @@ -1535,8 +1553,13 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return -ENOMEM; }
+ ret = devm_add_action_or_reset(dev, vc4_dsi_dma_mem_release, dsi); + if (ret) + return ret; + dma_cap_zero(dma_mask); dma_cap_set(DMA_MEMCPY, dma_mask); + dsi->reg_dma_chan = dma_request_chan_by_mask(&dma_mask); if (IS_ERR(dsi->reg_dma_chan)) { ret = PTR_ERR(dsi->reg_dma_chan); @@ -1546,6 +1569,10 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return ret; }
+ ret = devm_add_action_or_reset(dev, vc4_dsi_dma_chan_release, dsi); + if (ret) + return ret; + /* Get the physical address of the device's registers. The * struct resource for the regs gives us the bus address * instead.
From: Dave Stevenson dave.stevenson@raspberrypi.com
The divider calculations tried to find the divider just faster than the clock requested. However if it required a divider of 7 then the for loop aborted without handling the "error" case, and could end up with a clock lower than requested.
The integer divider from parent PLL to DSI clock is also capable of going up to /255, not just /7 that the driver was trying. This allows for slower link frequencies on the DSI bus where the resolution permits.
Correct the loop so that we always have a clock greater than requested, and covering the whole range of dividers.
Fixes: 86c1b9eff3f2 ("drm/vc4: Adjust modes in DSI to work around the integer PLL divider.") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index e82ee94cafc7..81a6c4e9576d 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -805,11 +805,9 @@ static bool vc4_dsi_encoder_mode_fixup(struct drm_encoder *encoder, /* Find what divider gets us a faster clock than the requested * pixel clock. */ - for (divider = 1; divider < 8; divider++) { - if (parent_rate / divider < pll_clock) { - divider--; + for (divider = 1; divider < 255; divider++) { + if (parent_rate / (divider + 1) < pll_clock) break; - } }
/* Now that we've picked a PLL divider, calculate back to its
From: Dave Stevenson dave.stevenson@raspberrypi.com
For slightly unknown reasons, dsi0 takes a different pixel format to dsi1, and that has to be set in the pixel valve.
Amend the setup accordingly.
Fixes: a86773d120d7 ("drm/vc4: Add support for feeding DSI encoders from the pixel valve.") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 59b20c8f132b..f74270ad3e13 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -319,7 +319,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode u32 pixel_rep = (mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1; bool is_dsi = (vc4_encoder->type == VC4_ENCODER_TYPE_DSI0 || vc4_encoder->type == VC4_ENCODER_TYPE_DSI1); - u32 format = is_dsi ? PV_CONTROL_FORMAT_DSIV_24 : PV_CONTROL_FORMAT_24; + bool is_dsi1 = vc4_encoder->type == VC4_ENCODER_TYPE_DSI1; + u32 format = is_dsi1 ? PV_CONTROL_FORMAT_DSIV_24 : PV_CONTROL_FORMAT_24; u8 ppc = pv_data->pixels_per_clock; bool debug_dump_regs = false;
From: Dave Stevenson dave.stevenson@raspberrypi.com
vc4_dsi was registering both dsi0 and dsi1 as VC4_ENCODER_TYPE_DSI1 which seemed to work OK for a single DSI display, but fails if there are two DSI displays connected.
Update to register the correct type.
Fixes: 4078f5757144 ("drm/vc4: Add DSI driver") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 81a6c4e9576d..97a258c934af 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1518,7 +1518,8 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return -ENOMEM;
INIT_LIST_HEAD(&dsi->bridge_chain); - vc4_dsi_encoder->base.type = VC4_ENCODER_TYPE_DSI1; + vc4_dsi_encoder->base.type = dsi->variant->port ? + VC4_ENCODER_TYPE_DSI1 : VC4_ENCODER_TYPE_DSI0; vc4_dsi_encoder->dsi = dsi; dsi->encoder = &vc4_dsi_encoder->base.base;
From: Dave Stevenson dave.stevenson@raspberrypi.com
DSI0 seemingly had very little or no testing as a load of the register mappings were incorrect/missing, so host transfers always timed out due to enabling/checking incorrect bits in the interrupt enable and status registers.
Fixes: 4078f5757144 ("drm/vc4: Add DSI driver") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 111 ++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 97a258c934af..333ea96fcde4 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -181,8 +181,50 @@
#define DSI0_TXPKT_PIX_FIFO 0x20 /* AKA PIX_FIFO */
-#define DSI0_INT_STAT 0x24 -#define DSI0_INT_EN 0x28 +#define DSI0_INT_STAT 0x24 +#define DSI0_INT_EN 0x28 +# define DSI0_INT_FIFO_ERR BIT(25) +# define DSI0_INT_CMDC_DONE_MASK VC4_MASK(24, 23) +# define DSI0_INT_CMDC_DONE_SHIFT 23 +# define DSI0_INT_CMDC_DONE_NO_REPEAT 1 +# define DSI0_INT_CMDC_DONE_REPEAT 3 +# define DSI0_INT_PHY_DIR_RTF BIT(22) +# define DSI0_INT_PHY_D1_ULPS BIT(21) +# define DSI0_INT_PHY_D1_STOP BIT(20) +# define DSI0_INT_PHY_RXLPDT BIT(19) +# define DSI0_INT_PHY_RXTRIG BIT(18) +# define DSI0_INT_PHY_D0_ULPS BIT(17) +# define DSI0_INT_PHY_D0_LPDT BIT(16) +# define DSI0_INT_PHY_D0_FTR BIT(15) +# define DSI0_INT_PHY_D0_STOP BIT(14) +/* Signaled when the clock lane enters the given state. */ +# define DSI0_INT_PHY_CLK_ULPS BIT(13) +# define DSI0_INT_PHY_CLK_HS BIT(12) +# define DSI0_INT_PHY_CLK_FTR BIT(11) +/* Signaled on timeouts */ +# define DSI0_INT_PR_TO BIT(10) +# define DSI0_INT_TA_TO BIT(9) +# define DSI0_INT_LPRX_TO BIT(8) +# define DSI0_INT_HSTX_TO BIT(7) +/* Contention on a line when trying to drive the line low */ +# define DSI0_INT_ERR_CONT_LP1 BIT(6) +# define DSI0_INT_ERR_CONT_LP0 BIT(5) +/* Control error: incorrect line state sequence on data lane 0. */ +# define DSI0_INT_ERR_CONTROL BIT(4) +# define DSI0_INT_ERR_SYNC_ESC BIT(3) +# define DSI0_INT_RX2_PKT BIT(2) +# define DSI0_INT_RX1_PKT BIT(1) +# define DSI0_INT_CMD_PKT BIT(0) + +#define DSI0_INTERRUPTS_ALWAYS_ENABLED (DSI0_INT_ERR_SYNC_ESC | \ + DSI0_INT_ERR_CONTROL | \ + DSI0_INT_ERR_CONT_LP0 | \ + DSI0_INT_ERR_CONT_LP1 | \ + DSI0_INT_HSTX_TO | \ + DSI0_INT_LPRX_TO | \ + DSI0_INT_TA_TO | \ + DSI0_INT_PR_TO) + # define DSI1_INT_PHY_D3_ULPS BIT(30) # define DSI1_INT_PHY_D3_STOP BIT(29) # define DSI1_INT_PHY_D2_ULPS BIT(28) @@ -892,6 +934,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
DSI_PORT_WRITE(PHY_AFEC0, afec0);
+ /* AFEC reset hold time */ + mdelay(1); + DSI_PORT_WRITE(PHY_AFEC1, VC4_SET_FIELD(6, DSI0_PHY_AFEC1_IDR_DLANE1) | VC4_SET_FIELD(6, DSI0_PHY_AFEC1_IDR_DLANE0) | @@ -1058,12 +1103,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DSI_PORT_WRITE(CTRL, DSI_PORT_READ(CTRL) | DSI1_CTRL_EN);
/* Bring AFE out of reset. */ - if (dsi->variant->port == 0) { - } else { - DSI_PORT_WRITE(PHY_AFEC0, - DSI_PORT_READ(PHY_AFEC0) & - ~DSI1_PHY_AFEC0_RESET); - } + DSI_PORT_WRITE(PHY_AFEC0, + DSI_PORT_READ(PHY_AFEC0) & + ~DSI_PORT_BIT(PHY_AFEC0_RESET));
vc4_dsi_ulps(dsi, false);
@@ -1182,13 +1224,28 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host, /* Enable the appropriate interrupt for the transfer completion. */ dsi->xfer_result = 0; reinit_completion(&dsi->xfer_completion); - DSI_PORT_WRITE(INT_STAT, DSI1_INT_TXPKT1_DONE | DSI1_INT_PHY_DIR_RTF); - if (msg->rx_len) { - DSI_PORT_WRITE(INT_EN, (DSI1_INTERRUPTS_ALWAYS_ENABLED | - DSI1_INT_PHY_DIR_RTF)); + if (dsi->variant->port == 0) { + DSI_PORT_WRITE(INT_STAT, + DSI0_INT_CMDC_DONE_MASK | DSI1_INT_PHY_DIR_RTF); + if (msg->rx_len) { + DSI_PORT_WRITE(INT_EN, (DSI0_INTERRUPTS_ALWAYS_ENABLED | + DSI0_INT_PHY_DIR_RTF)); + } else { + DSI_PORT_WRITE(INT_EN, + (DSI0_INTERRUPTS_ALWAYS_ENABLED | + VC4_SET_FIELD(DSI0_INT_CMDC_DONE_NO_REPEAT, + DSI0_INT_CMDC_DONE))); + } } else { - DSI_PORT_WRITE(INT_EN, (DSI1_INTERRUPTS_ALWAYS_ENABLED | - DSI1_INT_TXPKT1_DONE)); + DSI_PORT_WRITE(INT_STAT, + DSI1_INT_TXPKT1_DONE | DSI1_INT_PHY_DIR_RTF); + if (msg->rx_len) { + DSI_PORT_WRITE(INT_EN, (DSI1_INTERRUPTS_ALWAYS_ENABLED | + DSI1_INT_PHY_DIR_RTF)); + } else { + DSI_PORT_WRITE(INT_EN, (DSI1_INTERRUPTS_ALWAYS_ENABLED | + DSI1_INT_TXPKT1_DONE)); + } }
/* Send the packet. */ @@ -1205,7 +1262,7 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host, ret = dsi->xfer_result; }
- DSI_PORT_WRITE(INT_EN, DSI1_INTERRUPTS_ALWAYS_ENABLED); + DSI_PORT_WRITE(INT_EN, DSI_PORT_BIT(INTERRUPTS_ALWAYS_ENABLED));
if (ret) goto reset_fifo_and_return; @@ -1251,7 +1308,7 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host, DSI_PORT_BIT(CTRL_RESET_FIFOS));
DSI_PORT_WRITE(TXPKT1C, 0); - DSI_PORT_WRITE(INT_EN, DSI1_INTERRUPTS_ALWAYS_ENABLED); + DSI_PORT_WRITE(INT_EN, DSI_PORT_BIT(INTERRUPTS_ALWAYS_ENABLED)); return ret; }
@@ -1388,26 +1445,28 @@ static irqreturn_t vc4_dsi_irq_handler(int irq, void *data) DSI_PORT_WRITE(INT_STAT, stat);
dsi_handle_error(dsi, &ret, stat, - DSI1_INT_ERR_SYNC_ESC, "LPDT sync"); + DSI_PORT_BIT(INT_ERR_SYNC_ESC), "LPDT sync"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_ERR_CONTROL, "data lane 0 sequence"); + DSI_PORT_BIT(INT_ERR_CONTROL), "data lane 0 sequence"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_ERR_CONT_LP0, "LP0 contention"); + DSI_PORT_BIT(INT_ERR_CONT_LP0), "LP0 contention"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_ERR_CONT_LP1, "LP1 contention"); + DSI_PORT_BIT(INT_ERR_CONT_LP1), "LP1 contention"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_HSTX_TO, "HSTX timeout"); + DSI_PORT_BIT(INT_HSTX_TO), "HSTX timeout"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_LPRX_TO, "LPRX timeout"); + DSI_PORT_BIT(INT_LPRX_TO), "LPRX timeout"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_TA_TO, "turnaround timeout"); + DSI_PORT_BIT(INT_TA_TO), "turnaround timeout"); dsi_handle_error(dsi, &ret, stat, - DSI1_INT_PR_TO, "peripheral reset timeout"); + DSI_PORT_BIT(INT_PR_TO), "peripheral reset timeout");
- if (stat & (DSI1_INT_TXPKT1_DONE | DSI1_INT_PHY_DIR_RTF)) { + if (stat & ((dsi->variant->port ? DSI1_INT_TXPKT1_DONE : + DSI0_INT_CMDC_DONE_MASK) | + DSI_PORT_BIT(INT_PHY_DIR_RTF))) { complete(&dsi->xfer_completion); ret = IRQ_HANDLED; - } else if (stat & DSI1_INT_HSTX_TO) { + } else if (stat & DSI_PORT_BIT(INT_HSTX_TO)) { complete(&dsi->xfer_completion); dsi->xfer_result = -ETIMEDOUT; ret = IRQ_HANDLED;
From: Dave Stevenson dave.stevenson@raspberrypi.com
vc4_dsi_encoder_disable is partially an open coded version of drm_bridge_chain_disable, but it missed a termination condition in the loop for ->disable which meant that no post_disable calls were made.
Add in the termination clause.
Fixes: 033bfe7538a1 ("drm/vc4: dsi: Fix bridge chain handling") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 333ea96fcde4..b7b2c76770dc 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -803,6 +803,9 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder) list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { if (iter->funcs->disable) iter->funcs->disable(iter); + + if (iter == dsi->bridge) + break; }
vc4_dsi_ulps(dsi, true);
From: Phil Elwell phil@raspberrypi.org
The dmas property is used to hold the dmaengine channel used for audio output.
Older device trees were missing that property, so if it's not there we disable the audio output entirely.
However, some overlays have set an empty value to that property, mostly to workaround the fact that overlays cannot remove a property. Let's add a test for that case and if it's empty, let's disable it as well.
Cc: stable@vger.kernel.org Signed-off-by: Phil Elwell phil@raspberrypi.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 6aadb65eb640..c8571e17afa8 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2034,12 +2034,12 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) struct device *dev = &vc4_hdmi->pdev->dev; struct platform_device *codec_pdev; const __be32 *addr; - int index; + int index, len; int ret;
- if (!of_find_property(dev->of_node, "dmas", NULL)) { + if (!of_find_property(dev->of_node, "dmas", &len) || !len) { dev_warn(dev, - "'dmas' DT property is missing, no HDMI audio\n"); + "'dmas' DT property is missing or empty, no HDMI audio\n"); return 0; }
From: Dave Stevenson dave.stevenson@raspberrypi.com
The vc5 HDMI registers hadn't been added into the debugfs register sets, therefore weren't dumped on request. Add them in.
Fixes: 8323989140f3 ("drm/vc4: hdmi: Support the BCM2711 HDMI controllers") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 39 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi.h | 8 +++++++ 2 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index c8571e17afa8..d23ed6e5bd65 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -144,6 +144,12 @@ static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
drm_print_regset32(&p, &vc4_hdmi->hdmi_regset); drm_print_regset32(&p, &vc4_hdmi->hd_regset); + drm_print_regset32(&p, &vc4_hdmi->cec_regset); + drm_print_regset32(&p, &vc4_hdmi->csc_regset); + drm_print_regset32(&p, &vc4_hdmi->dvp_regset); + drm_print_regset32(&p, &vc4_hdmi->phy_regset); + drm_print_regset32(&p, &vc4_hdmi->ram_regset); + drm_print_regset32(&p, &vc4_hdmi->rm_regset);
return 0; } @@ -2703,6 +2709,7 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) struct platform_device *pdev = vc4_hdmi->pdev; struct device *dev = &pdev->dev; struct resource *res; + int ret;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hdmi"); if (!res) @@ -2799,6 +2806,38 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return PTR_ERR(vc4_hdmi->reset); }
+ ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->cec_regset, VC5_CEC); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->csc_regset, VC5_CSC); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->dvp_regset, VC5_DVP); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->phy_regset, VC5_PHY); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->ram_regset, VC5_RAM); + if (ret) + return ret; + + ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->rm_regset, VC5_RM); + if (ret) + return ret; + return 0; }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 51b27dcdcd9b..1520387b317f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -179,6 +179,14 @@ struct vc4_hdmi { struct debugfs_regset32 hdmi_regset; struct debugfs_regset32 hd_regset;
+ /* VC5 only */ + struct debugfs_regset32 cec_regset; + struct debugfs_regset32 csc_regset; + struct debugfs_regset32 dvp_regset; + struct debugfs_regset32 phy_regset; + struct debugfs_regset32 ram_regset; + struct debugfs_regset32 rm_regset; + /** * @hw_lock: Spinlock protecting device register access. */
From: Dom Cobley popcornmix@gmail.com
Using a hdmi analyser the bytes in packet ram registers beyond the length were visible in the infoframes and it flagged the checksum as invalid.
Zeroing unused words of packet RAM avoids this
Fixes: 21317b3fba54 ("drm/vc4: Set up the AVI and SPD infoframes.") Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index d23ed6e5bd65..4b73b4fea7ec 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -460,9 +460,11 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, const struct vc4_hdmi_register *ram_packet_start = &vc4_hdmi->variant->registers[HDMI_RAM_PACKET_START]; u32 packet_reg = ram_packet_start->offset + VC4_HDMI_PACKET_STRIDE * packet_id; + u32 packet_reg_next = ram_packet_start->offset + + VC4_HDMI_PACKET_STRIDE * (packet_id + 1); void __iomem *base = __vc4_hdmi_get_field_base(vc4_hdmi, ram_packet_start->reg); - uint8_t buffer[VC4_HDMI_PACKET_STRIDE]; + uint8_t buffer[VC4_HDMI_PACKET_STRIDE] = {}; unsigned long flags; ssize_t len, i; int ret; @@ -498,6 +500,13 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, packet_reg += 4; }
+ /* + * clear remainder of packet ram as it's included in the + * infoframe and triggers a checksum error on hdmi analyser + */ + for (; packet_reg < packet_reg_next; packet_reg += 4) + writel(0, base + packet_reg); + HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, HDMI_READ(HDMI_RAM_PACKET_CONFIG) | BIT(packet_id));
From: Dom Cobley popcornmix@gmail.com
We are getting occasional VC4_HD_MAI_CTL_ERRORF in HDMI_MAI_CTL which seem to correspond with audio dropouts.
Reduce the threshold where we deassert DREQ to avoid the fifo overfilling
Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support") Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4b73b4fea7ec..53cc0b7b664c 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1955,10 +1955,10 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data,
/* Set the MAI threshold */ HDMI_WRITE(HDMI_MAI_THR, - VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICHIGH) | - VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_PANICLOW) | - VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQHIGH) | - VC4_SET_FIELD(0x10, VC4_HD_MAI_THR_DREQLOW)); + VC4_SET_FIELD(0x08, VC4_HD_MAI_THR_PANICHIGH) | + VC4_SET_FIELD(0x08, VC4_HD_MAI_THR_PANICLOW) | + VC4_SET_FIELD(0x06, VC4_HD_MAI_THR_DREQHIGH) | + VC4_SET_FIELD(0x08, VC4_HD_MAI_THR_DREQLOW));
HDMI_WRITE(HDMI_MAI_CONFIG, VC4_HDMI_MAI_CONFIG_BIT_REVERSE |
From: Dave Stevenson dave.stevenson@raspberrypi.com
The HDMI block can repeat pixels for double clocked modes, and the firmware is now configuring the block to do this as the PV is doing it incorrectly when at 2pixels/clock. If the kernel doesn't reset it then we end up with strange modes.
Reset MISC_CONTROL.
Fixes: 8323989140f3 ("drm/vc4: hdmi: Support the BCM2711 HDMI controllers") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 8 ++++++++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 53cc0b7b664c..8142efa2d479 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -78,6 +78,9 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_MISC_CONTROL_PIXEL_REP_SHIFT 0 +#define VC5_HDMI_MISC_CONTROL_PIXEL_REP_MASK VC4_MASK(3, 0) + #define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 @@ -1116,6 +1119,11 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, reg |= gcp_en ? VC5_HDMI_GCP_CONFIG_GCP_ENABLE : 0; HDMI_WRITE(HDMI_GCP_CONFIG, reg);
+ reg = HDMI_READ(HDMI_MISC_CONTROL); + reg &= ~VC5_HDMI_MISC_CONTROL_PIXEL_REP_MASK; + reg |= VC4_SET_FIELD(0, VC5_HDMI_MISC_CONTROL_PIXEL_REP); + HDMI_WRITE(HDMI_MISC_CONTROL, reg); + HDMI_WRITE(HDMI_CLOCK_STOP, 0);
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index a040356b6bdc..549cc63dab39 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -127,6 +127,7 @@ enum vc4_hdmi_field { HDMI_VERTB0, HDMI_VERTB1, HDMI_VID_CTL, + HDMI_MISC_CONTROL, };
struct vc4_hdmi_register { @@ -237,6 +238,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_VERTB0, 0x0f0), VC4_HDMI_REG(HDMI_VERTA1, 0x0f4), VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), + VC4_HDMI_REG(HDMI_MISC_CONTROL, 0x100), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170), @@ -319,6 +321,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_VERTB0, 0x0f0), VC4_HDMI_REG(HDMI_VERTA1, 0x0f4), VC4_HDMI_REG(HDMI_VERTB1, 0x0f8), + VC4_HDMI_REG(HDMI_MISC_CONTROL, 0x100), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170),
From: Dave Stevenson dave.stevenson@raspberrypi.com
If the controller isn't clocked or its domain powered up, the register accesses will either stall the CPU or return garbage, respectively.
Thus, we had a warning in our register access function to complain when that kind of risky accesses were performed.
In order to check the runtime_pm power state, we were using pm_runtime_active(), but it turns out that it will become active only once the runtime_resume hook has been executed.
This prevents us from doing any WARN-free register access in our runtime_resume() implementation, while this is valid.
Let's switch to pm_runtime_status_suspended() instead.
Fixes: 14e193b95604 ("drm/vc4: hdmi: Warn if we access the controller while disabled") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 549cc63dab39..0198de96c7b2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -423,7 +423,7 @@ static inline u32 vc4_hdmi_read(struct vc4_hdmi *hdmi, const struct vc4_hdmi_variant *variant = hdmi->variant; void __iomem *base;
- WARN_ON(!pm_runtime_active(&hdmi->pdev->dev)); + WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev));
if (reg >= variant->num_registers) { dev_warn(&hdmi->pdev->dev, @@ -453,7 +453,7 @@ static inline void vc4_hdmi_write(struct vc4_hdmi *hdmi,
lockdep_assert_held(&hdmi->hw_lock);
- WARN_ON(!pm_runtime_active(&hdmi->pdev->dev)); + WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev));
if (reg >= variant->num_registers) { dev_warn(&hdmi->pdev->dev,
From: Dave Stevenson dave.stevenson@raspberrypi.com
The BCM2835-37 found in the RaspberryPi 0 to 3 have a power domain attached to the HDMI block, handled in Linux through runtime_pm.
That power domain is shared with the VEC block, so even if we put our runtime_pm reference in the HDMI driver it would keep being on. If the VEC is disabled though, the power domain would be disabled and we would lose any initialization done in our bind implementation.
That initialization involves calling the reset function and initializing the CEC registers.
Let's move the initialization to our runtime_resume implementation so that we initialize everything properly if we ever need to.
Fixes: c86b41214362 ("drm/vc4: hdmi: Move the HSM clock enable to runtime_pm") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 41 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 8142efa2d479..654c4116b669 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2543,8 +2543,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) struct cec_connector_info conn_info; struct platform_device *pdev = vc4_hdmi->pdev; struct device *dev = &pdev->dev; - unsigned long flags; - u32 value; int ret;
if (!of_find_property(dev->of_node, "interrupts", NULL)) { @@ -2563,15 +2561,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) cec_fill_conn_info_from_drm(&conn_info, &vc4_hdmi->connector); cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
- spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); - value = HDMI_READ(HDMI_CEC_CNTRL_1); - /* Set the logical address to Unregistered */ - value |= VC4_HDMI_CEC_ADDR_MASK; - HDMI_WRITE(HDMI_CEC_CNTRL_1, value); - spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); - - vc4_hdmi_cec_update_clk_div(vc4_hdmi); - if (vc4_hdmi->variant->external_irq_controller) { ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_cec_irq_handler_rx_bare, @@ -2587,10 +2576,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) if (ret) goto err_remove_cec_rx_handler; } else { - spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); - HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff); - spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); - ret = request_threaded_irq(platform_get_irq(pdev, 0), vc4_cec_irq_handler, vc4_cec_irq_handler_thread, 0, @@ -2641,7 +2626,6 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) }
static void vc4_hdmi_cec_exit(struct vc4_hdmi *vc4_hdmi) {}; - #endif
static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi, @@ -2870,12 +2854,34 @@ static int __maybe_unused vc4_hdmi_runtime_suspend(struct device *dev) static int vc4_hdmi_runtime_resume(struct device *dev) { struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev); + unsigned long __maybe_unused flags; + u32 __maybe_unused value; int ret;
ret = clk_prepare_enable(vc4_hdmi->hsm_clock); if (ret) return ret;
+ if (vc4_hdmi->variant->reset) + vc4_hdmi->variant->reset(vc4_hdmi); + +#ifdef CONFIG_DRM_VC4_HDMI_CEC + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); + value = HDMI_READ(HDMI_CEC_CNTRL_1); + /* Set the logical address to Unregistered */ + value |= VC4_HDMI_CEC_ADDR_MASK; + HDMI_WRITE(HDMI_CEC_CNTRL_1, value); + spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + vc4_hdmi_cec_update_clk_div(vc4_hdmi); + + if (!vc4_hdmi->variant->external_irq_controller) { + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); + HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff); + spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + } +#endif + return 0; }
@@ -2965,9 +2971,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) pm_runtime_set_active(dev); pm_runtime_enable(dev);
- if (vc4_hdmi->variant->reset) - vc4_hdmi->variant->reset(vc4_hdmi); - if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") || of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) && HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) {
From: Dom Cobley popcornmix@gmail.com
The current HDMI driver, in vc4_hdmi_audio_can_stream() checks whether the display output is enabled.
This has been there in one form or the other since the introduction of the audio support in the VC4 HDMI driver in commit bb7d78568814 ("drm/vc4: Add HDMI audio support"), but no justification for this check is in the commit message, or in the discussions around the patches.
One can only assume this was done to prevent a user from playing audio on the ALSA soundcard when the monitor doesn't support it.
However, this is causing some issues. Indeed, Kodi, for example, was hitting some errors if it was streaming audio during a modeset. With the theory above, it does make sense, but the display and audio threads are typically completely different processes with no opportunity to synchronise which makes it hard to workaround.
Removing that check also doesn't seem to cause any trouble, so let's just remove it.
Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 30 +++--------------------------- drivers/gpu/drm/vc4/vc4_hdmi.h | 6 ------ 2 files changed, 3 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 654c4116b669..0dc7e57715af 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -788,15 +788,6 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, mutex_unlock(&vc4_hdmi->mutex); }
-static void vc4_hdmi_encoder_disable(struct drm_encoder *encoder) -{ - struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); - - mutex_lock(&vc4_hdmi->mutex); - vc4_hdmi->output_enabled = false; - mutex_unlock(&vc4_hdmi->mutex); -} - static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, const struct drm_display_mode *mode) @@ -1370,15 +1361,6 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, mutex_unlock(&vc4_hdmi->mutex); }
-static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder) -{ - struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); - - mutex_lock(&vc4_hdmi->mutex); - vc4_hdmi->output_enabled = true; - mutex_unlock(&vc4_hdmi->mutex); -} - static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) @@ -1672,8 +1654,6 @@ static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = { .atomic_check = vc4_hdmi_encoder_atomic_check, .atomic_mode_set = vc4_hdmi_encoder_atomic_mode_set, .mode_valid = vc4_hdmi_encoder_mode_valid, - .disable = vc4_hdmi_encoder_disable, - .enable = vc4_hdmi_encoder_enable, };
static u32 vc4_hdmi_channel_map(struct vc4_hdmi *vc4_hdmi, u32 channel_mask) @@ -1770,19 +1750,15 @@ static inline struct vc4_hdmi *dai_to_hdmi(struct snd_soc_dai *dai)
static bool vc4_hdmi_audio_can_stream(struct vc4_hdmi *vc4_hdmi) { - lockdep_assert_held(&vc4_hdmi->mutex); + struct drm_display_info *display = &vc4_hdmi->connector.display_info;
- /* - * If the controller is disabled, prevent any ALSA output. - */ - if (!vc4_hdmi->output_enabled) - return false; + lockdep_assert_held(&vc4_hdmi->mutex);
/* * If the encoder is currently in DVI mode, treat the codec DAI * as missing. */ - if (!(HDMI_READ(HDMI_RAM_PACKET_CONFIG) & VC4_HDMI_RAM_PACKET_ENABLE)) + if (!display->is_hdmi) return false;
return true; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 1520387b317f..1159b2992fb1 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -212,12 +212,6 @@ struct vc4_hdmi { */ struct drm_display_mode saved_adjusted_mode;
- /** - * @output_enabled: Is the HDMI controller currently active? - * Protected by @mutex. - */ - bool output_enabled; - /** * @scdc_enabled: Is the HDMI controller currently running with * the scrambler on? Protected by @mutex.
From: Dom Cobley popcornmix@gmail.com
This path actually occurs when audio is started during a hdmi mode set. As the data will be written by vc4_hdmi_set_infoframes when packet RAM is enabled again, don't treat as an error
Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ++++++- drivers/gpu/drm/vc4/vc4_hdmi.h | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 0dc7e57715af..c62e32f68974 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -601,7 +601,9 @@ static void vc4_hdmi_set_audio_infoframe(struct drm_encoder *encoder) union hdmi_infoframe frame;
memcpy(&frame.audio, audio, sizeof(*audio)); - vc4_hdmi_write_infoframe(encoder, &frame); + + if (vc4_hdmi->packet_ram_enabled) + vc4_hdmi_write_infoframe(encoder, &frame); }
static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder) @@ -741,6 +743,8 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
mutex_lock(&vc4_hdmi->mutex);
+ vc4_hdmi->packet_ram_enabled = false; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, 0); @@ -1351,6 +1355,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, VC4_HDMI_RAM_PACKET_ENABLE);
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + vc4_hdmi->packet_ram_enabled = true;
vc4_hdmi_set_infoframes(encoder); } diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 1159b2992fb1..c3ed2b07df23 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -212,6 +212,12 @@ struct vc4_hdmi { */ struct drm_display_mode saved_adjusted_mode;
+ /** + * @packet_ram_enabled: Is the HDMI controller packet RAM currently + * on? Protected by @mutex. + */ + bool packet_ram_enabled; + /** * @scdc_enabled: Is the HDMI controller currently running with * the scrambler on? Protected by @mutex.
From: Dom Cobley popcornmix@gmail.com
This bit ensures data island packets are never generated when disallowed by HDCP. As no Pi boards support HDCP this is providing an unnecessary restriction
Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index c62e32f68974..fd5ff2a9bd6c 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1347,9 +1347,6 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
WARN_ON(!(HDMI_READ(HDMI_SCHEDULER_CONTROL) & VC4_HDMI_SCHEDULER_CONTROL_HDMI_ACTIVE)); - HDMI_WRITE(HDMI_SCHEDULER_CONTROL, - HDMI_READ(HDMI_SCHEDULER_CONTROL) | - VC4_HDMI_SCHEDULER_CONTROL_VERT_ALWAYS_KEEPOUT);
HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, VC4_HDMI_RAM_PACKET_ENABLE);
From: Dave Stevenson dave.stevenson@raspberrypi.com
The block can detect what the incoming image timings are for debug purposes. Add them to the list of registers understood by the driver to allow easy dumping of the values.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 30 +++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 0198de96c7b2..5a56761e75af 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -128,6 +128,16 @@ enum vc4_hdmi_field { HDMI_VERTB1, HDMI_VID_CTL, HDMI_MISC_CONTROL, + HDMI_FORMAT_DET_1, + HDMI_FORMAT_DET_2, + HDMI_FORMAT_DET_3, + HDMI_FORMAT_DET_4, + HDMI_FORMAT_DET_5, + HDMI_FORMAT_DET_6, + HDMI_FORMAT_DET_7, + HDMI_FORMAT_DET_8, + HDMI_FORMAT_DET_9, + HDMI_FORMAT_DET_10, };
struct vc4_hdmi_register { @@ -241,6 +251,16 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_MISC_CONTROL, 0x100), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), + VC4_HDMI_REG(HDMI_FORMAT_DET_1, 0x134), + VC4_HDMI_REG(HDMI_FORMAT_DET_2, 0x138), + VC4_HDMI_REG(HDMI_FORMAT_DET_3, 0x13c), + VC4_HDMI_REG(HDMI_FORMAT_DET_4, 0x140), + VC4_HDMI_REG(HDMI_FORMAT_DET_5, 0x144), + VC4_HDMI_REG(HDMI_FORMAT_DET_6, 0x148), + VC4_HDMI_REG(HDMI_FORMAT_DET_7, 0x14c), + VC4_HDMI_REG(HDMI_FORMAT_DET_8, 0x150), + VC4_HDMI_REG(HDMI_FORMAT_DET_9, 0x154), + VC4_HDMI_REG(HDMI_FORMAT_DET_10, 0x158), VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170), VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), @@ -324,6 +344,16 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_MISC_CONTROL, 0x100), VC4_HDMI_REG(HDMI_MAI_CHANNEL_MAP, 0x09c), VC4_HDMI_REG(HDMI_MAI_CONFIG, 0x0a0), + VC4_HDMI_REG(HDMI_FORMAT_DET_1, 0x134), + VC4_HDMI_REG(HDMI_FORMAT_DET_2, 0x138), + VC4_HDMI_REG(HDMI_FORMAT_DET_3, 0x13c), + VC4_HDMI_REG(HDMI_FORMAT_DET_4, 0x140), + VC4_HDMI_REG(HDMI_FORMAT_DET_5, 0x144), + VC4_HDMI_REG(HDMI_FORMAT_DET_6, 0x148), + VC4_HDMI_REG(HDMI_FORMAT_DET_7, 0x14c), + VC4_HDMI_REG(HDMI_FORMAT_DET_8, 0x150), + VC4_HDMI_REG(HDMI_FORMAT_DET_9, 0x154), + VC4_HDMI_REG(HDMI_FORMAT_DET_10, 0x158), VC4_HDMI_REG(HDMI_DEEP_COLOR_CONFIG_1, 0x170), VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c),
From: Dave Stevenson dave.stevenson@raspberrypi.com
The MISC_CONTROL register allows configuration of pixel repetition for pixel doubling in the HDMI block instead of PixelValve. It was already defined for vc5, so add it for vc4.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 5a56761e75af..48db438550b1 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -200,6 +200,7 @@ static const struct vc4_hdmi_register __maybe_unused vc4_hdmi_fields[] = { VC4_HDMI_REG(HDMI_VERTB0, 0x00d0), VC4_HDMI_REG(HDMI_VERTA1, 0x00d4), VC4_HDMI_REG(HDMI_VERTB1, 0x00d8), + VC4_HDMI_REG(HDMI_MISC_CONTROL, 0x00e4), VC4_HDMI_REG(HDMI_CEC_CNTRL_1, 0x00e8), VC4_HDMI_REG(HDMI_CEC_CNTRL_2, 0x00ec), VC4_HDMI_REG(HDMI_CEC_CNTRL_3, 0x00f0),
From: Dom Cobley popcornmix@gmail.com
Our HDMI controllers supports Stereo output so let's enable it.
Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index fd5ff2a9bd6c..95974f757b47 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -426,6 +426,7 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1; connector->doublescan_allowed = 0; + connector->stereo_allowed = 1;
if (vc4_hdmi->variant->supports_hdr) drm_connector_attach_hdr_output_metadata_property(connector);
From: Mateusz Kwiatkowski kfyatek+publicgit@gmail.com
Increase the number of post-sync blanking lines on odd fields instead of decreasing it on even fields. This makes the total number of lines properly match the modelines.
Additionally fix the value of PV_VCONTROL_ODD_DELAY, which did not take pixels_per_clock into account, causing some displays to invert the fields when driven by bcm2711.
Fixes: 682e62c45406 ("drm/vc4: Fix support for interlaced modes on HDMI.") Signed-off-by: Mateusz Kwiatkowski kfyatek+publicgit@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 7 ++++--- drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++------ 2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index f74270ad3e13..e4e8c8a4c804 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -346,7 +346,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode PV_HORZB_HACTIVE));
CRTC_WRITE(PV_VERTA, - VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end, + VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end + + interlace, PV_VERTA_VBP) | VC4_SET_FIELD(mode->crtc_vsync_end - mode->crtc_vsync_start, PV_VERTA_VSYNC)); @@ -358,7 +359,7 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode if (interlace) { CRTC_WRITE(PV_VERTA_EVEN, VC4_SET_FIELD(mode->crtc_vtotal - - mode->crtc_vsync_end - 1, + mode->crtc_vsync_end, PV_VERTA_VBP) | VC4_SET_FIELD(mode->crtc_vsync_end - mode->crtc_vsync_start, @@ -378,7 +379,7 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode PV_VCONTROL_CONTINUOUS | (is_dsi ? PV_VCONTROL_DSI : 0) | PV_VCONTROL_INTERLACE | - VC4_SET_FIELD(mode->htotal * pixel_rep / 2, + VC4_SET_FIELD(mode->htotal * pixel_rep / (2 * ppc), PV_VCONTROL_ODD_DELAY)); CRTC_WRITE(PV_VSYNCD_EVEN, 0); } else { diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 95974f757b47..305807791ebd 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -983,12 +983,12 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, VC4_HDMI_VERTA_VFP) | VC4_SET_FIELD(mode->crtc_vdisplay, VC4_HDMI_VERTA_VAL)); u32 vertb = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) | - VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end, + VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end + + interlaced, VC4_HDMI_VERTB_VBP)); u32 vertb_even = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) | VC4_SET_FIELD(mode->crtc_vtotal - - mode->crtc_vsync_end - - interlaced, + mode->crtc_vsync_end, VC4_HDMI_VERTB_VBP)); unsigned long flags;
@@ -1036,12 +1036,12 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, VC5_HDMI_VERTA_VFP) | VC4_SET_FIELD(mode->crtc_vdisplay, VC5_HDMI_VERTA_VAL)); u32 vertb = (VC4_SET_FIELD(0, VC5_HDMI_VERTB_VSPO) | - VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end, + VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end + + interlaced, VC4_HDMI_VERTB_VBP)); u32 vertb_even = (VC4_SET_FIELD(0, VC5_HDMI_VERTB_VSPO) | VC4_SET_FIELD(mode->crtc_vtotal - - mode->crtc_vsync_end - - interlaced, + mode->crtc_vsync_end, VC4_HDMI_VERTB_VBP)); unsigned long flags; unsigned char gcp;
From: Dom Cobley popcornmix@gmail.com
Whenever the maximum BPC is changed, vc4_hdmi_encoder_compute_config() might pick up a different BPC or format depending on the display capabilities.
That change will have a number of side effects, including the clock rates and whether the scrambling is enabled.
However, only drm_crtc_state.connectors_changed will be set to true, since that properly only affects the connector.
This means that while drm_atomic_crtc_needs_modeset() will return true, and thus drm_atomic_helper_commit_modeset_enables() will call our encoder atomic_enable() hook, mode_changed will be false.
So crtc_set_mode() will not call our encoder .atomic_mode_set() hook. We use this hook in vc4 to set the vc4_hdmi_connector_state.output_bpc (and output_format), and will then reuse the value in .atomic_enable() to select whether or not scrambling should be enabled.
However, since our clock rate is pre-computed during .atomic_check(), we end up with the clocks properly configured, but the scrambling disabled, leading to a blank screen.
Let's set mode_changed to true in our HDMI driver to force the update of output_bpc, and thus prevent the issue entirely.
Fixes: ba8c0faebbb0 ("drm/vc4: hdmi: Enable 10/12 bpc output") Signed-off-by: Dom Cobley popcornmix@gmail.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 305807791ebd..03fa2d4c1827 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1604,9 +1604,14 @@ 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 *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_connector *connector = &vc4_hdmi->connector; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(conn_state->state, connector); + struct vc4_hdmi_connector_state *old_vc4_state = + conn_state_to_vc4_hdmi_conn_state(old_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 tmds_char_rate = mode->clock * 1000; unsigned long long tmds_bit_rate; int ret; @@ -1635,6 +1640,11 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (ret) return ret;
+ /* vc4_hdmi_encoder_compute_config may have changed output_bpc and/or output_format */ + if (vc4_state->output_bpc != old_vc4_state->output_bpc || + vc4_state->output_format != old_vc4_state->output_format) + crtc_state->mode_changed = true; + return 0; }
From: Dave Stevenson dave.stevenson@raspberrypi.com
For interlaced modes the timings were not being correctly programmed into the HDMI block, so correct them.
Fixes: 8323989140f3 ("drm/vc4: hdmi: Support the BCM2711 HDMI controllers") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 03fa2d4c1827..75cba7edaf51 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1035,13 +1035,13 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, VC4_SET_FIELD(mode->crtc_vsync_start - mode->crtc_vdisplay, VC5_HDMI_VERTA_VFP) | VC4_SET_FIELD(mode->crtc_vdisplay, VC5_HDMI_VERTA_VAL)); - u32 vertb = (VC4_SET_FIELD(0, VC5_HDMI_VERTB_VSPO) | - VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end + - interlaced, + u32 vertb = (VC4_SET_FIELD(mode->htotal >> (2 - pixel_rep), + VC5_HDMI_VERTB_VSPO) | + VC4_SET_FIELD(mode->crtc_vtotal - mode->crtc_vsync_end, VC4_HDMI_VERTB_VBP)); u32 vertb_even = (VC4_SET_FIELD(0, VC5_HDMI_VERTB_VSPO) | VC4_SET_FIELD(mode->crtc_vtotal - - mode->crtc_vsync_end, + mode->crtc_vsync_end - interlaced, VC4_HDMI_VERTB_VBP)); unsigned long flags; unsigned char gcp;
From: Dave Stevenson dave.stevenson@raspberrypi.com
With the change to 2 pixels/clock, the pixel doubling in the PV results in doubling each pair of pixels, ie ABABCDCD instead of AABBCCDD.
Move the pixel doubling to the HDMI block, however this means that DBLCLK modes now fall foul of requiring even values for all the horizontal timing parameters. As both 480i and 576i fail this, attempt to fix up DBLCLK modes that have odd timings values.
Fixes: 8323989140f3 ("drm/vc4: hdmi: Support the BCM2711 HDMI controllers") Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 4 +++- drivers/gpu/drm/vc4/vc4_hdmi.c | 34 ++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index e4e8c8a4c804..bf8f84b87580 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -316,7 +316,9 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode struct drm_crtc_state *crtc_state = crtc->state; struct drm_display_mode *mode = &crtc_state->adjusted_mode; bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE; - u32 pixel_rep = (mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1; + bool is_hdmi = vc4_encoder->type == VC4_ENCODER_TYPE_HDMI0 || + vc4_encoder->type == VC4_ENCODER_TYPE_HDMI1; + u32 pixel_rep = ((mode->flags & DRM_MODE_FLAG_DBLCLK) && !is_hdmi) ? 2 : 1; bool is_dsi = (vc4_encoder->type == VC4_ENCODER_TYPE_DSI0 || vc4_encoder->type == VC4_ENCODER_TYPE_DSI1); bool is_dsi1 = vc4_encoder->type == VC4_ENCODER_TYPE_DSI1; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 75cba7edaf51..5075f9cbab02 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -78,6 +78,8 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC4_HDMI_MISC_CONTROL_PIXEL_REP_SHIFT 0 +#define VC4_HDMI_MISC_CONTROL_PIXEL_REP_MASK VC4_MASK(3, 0) #define VC5_HDMI_MISC_CONTROL_PIXEL_REP_SHIFT 0 #define VC5_HDMI_MISC_CONTROL_PIXEL_REP_MASK VC4_MASK(3, 0)
@@ -991,6 +993,7 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, mode->crtc_vsync_end, VC4_HDMI_VERTB_VBP)); unsigned long flags; + u32 reg;
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
@@ -1017,6 +1020,11 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_VERTB0, vertb_even); HDMI_WRITE(HDMI_VERTB1, vertb);
+ reg = HDMI_READ(HDMI_MISC_CONTROL); + reg &= ~VC4_HDMI_MISC_CONTROL_PIXEL_REP_MASK; + reg |= VC4_SET_FIELD(pixel_rep - 1, VC4_HDMI_MISC_CONTROL_PIXEL_REP); + HDMI_WRITE(HDMI_MISC_CONTROL, reg); + spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); }
@@ -1117,7 +1125,7 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi,
reg = HDMI_READ(HDMI_MISC_CONTROL); reg &= ~VC5_HDMI_MISC_CONTROL_PIXEL_REP_MASK; - reg |= VC4_SET_FIELD(0, VC5_HDMI_MISC_CONTROL_PIXEL_REP); + reg |= VC4_SET_FIELD(pixel_rep - 1, VC5_HDMI_MISC_CONTROL_PIXEL_REP); HDMI_WRITE(HDMI_MISC_CONTROL, reg);
HDMI_WRITE(HDMI_CLOCK_STOP, 0); @@ -1616,11 +1624,25 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, unsigned long long tmds_bit_rate; int ret;
- if (vc4_hdmi->variant->unsupported_odd_h_timings && - !(mode->flags & DRM_MODE_FLAG_DBLCLK) && - ((mode->hdisplay % 2) || (mode->hsync_start % 2) || - (mode->hsync_end % 2) || (mode->htotal % 2))) - return -EINVAL; + if (vc4_hdmi->variant->unsupported_odd_h_timings) { + if (mode->flags & DRM_MODE_FLAG_DBLCLK) { + /* Only try to fixup DBLCLK modes to get 480i and 576i + * working. + * A generic solution for all modes with odd horizontal + * timing values seems impossible based on trying to + * solve it for 1366x768 monitors. + */ + if ((mode->hsync_start - mode->hdisplay) & 1) + mode->hsync_start--; + if ((mode->hsync_end - mode->hsync_start) & 1) + mode->hsync_end--; + } + + /* Now check whether we still have odd values remaining */ + if ((mode->hdisplay % 2) || (mode->hsync_start % 2) || + (mode->hsync_end % 2) || (mode->htotal % 2)) + return -EINVAL; + }
/* * The 1440p@60 pixel rate is in the same range than the first
dri-devel@lists.freedesktop.org
-
Maxime Ripard