From 1fd4a5a36f9f10aaad5d9b1b329c2c057d80a0e5 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 17 Nov 2022 10:28:45 +0100 Subject: drm/connector: Rename legacy TV property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current tv_mode has driver-specific values that don't allow to easily share code using it, either at the userspace or kernel level. Since we're going to introduce a new, generic, property that fit the same purpose, let's rename this one to legacy_tv_mode to make it obvious we should move away from it. Acked-by: Thomas Zimmermann Reviewed-by: Lyude Paul # nouveau Reviewed-by: Noralf Trønnes Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-2-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index 92c07e31d632..e6043cf5d40e 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -274,7 +274,7 @@ static int vc4_vec_connector_get_modes(struct drm_connector *connector) struct drm_display_mode *mode; mode = drm_mode_duplicate(connector->dev, - vc4_vec_tv_modes[state->tv.mode].mode); + vc4_vec_tv_modes[state->tv.legacy_mode].mode); if (!mode) { DRM_ERROR("Failed to create a new display mode\n"); return -ENOMEM; @@ -312,7 +312,7 @@ static int vc4_vec_connector_init(struct drm_device *dev, struct vc4_vec *vec) drm_connector_helper_add(connector, &vc4_vec_connector_helper_funcs); drm_object_attach_property(&connector->base, - dev->mode_config.tv_mode_property, + dev->mode_config.legacy_tv_mode_property, VC4_VEC_TV_MODE_NTSC); drm_connector_attach_encoder(connector, &vec->encoder.base); @@ -361,7 +361,7 @@ static void vc4_vec_encoder_enable(struct drm_encoder *encoder, struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); const struct vc4_vec_tv_mode *tv_mode = - &vc4_vec_tv_modes[conn_state->tv.mode]; + &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; int idx, ret; if (!drm_dev_enter(drm, &idx)) @@ -449,7 +449,7 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, { const struct vc4_vec_tv_mode *vec_mode; - vec_mode = &vc4_vec_tv_modes[conn_state->tv.mode]; + vec_mode = &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; if (conn_state->crtc && !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode)) -- cgit v1.2.3 From 80ed86d4b6d7cf91f4fd588bd7be2fa382724d2d Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 17 Nov 2022 10:28:47 +0100 Subject: drm/connector: Rename drm_mode_create_tv_properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drm_mode_create_tv_properties(), among other things, will create the "mode" property that stores the analog TV mode that connector is supposed to output. However, that property is getting deprecated, so let's rename that function to mention it's deprecated. We'll introduce a new variant of that function creating the property superseeding it in a later patch. Reviewed-by: Lyude Paul # nouveau Reviewed-by: Noralf Trønnes Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-4-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_connector.c | 12 ++++++------ drivers/gpu/drm/gud/gud_connector.c | 4 ++-- drivers/gpu/drm/i2c/ch7006_drv.c | 2 +- drivers/gpu/drm/i915/display/intel_tv.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- drivers/gpu/drm/vc4/vc4_vec.c | 5 +++-- include/drm/drm_connector.h | 6 +++--- 7 files changed, 17 insertions(+), 16 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 78fcffae100b..06e737ed15f5 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1604,7 +1604,7 @@ EXPORT_SYMBOL(drm_connector_attach_tv_margin_properties); * Called by a driver's HDMI connector initialization routine, this function * creates the TV margin properties for a given device. No need to call this * function for an SDTV connector, it's already called from - * drm_mode_create_tv_properties(). + * drm_mode_create_tv_properties_legacy(). * * Returns: * 0 on success or a negative error code on failure. @@ -1639,7 +1639,7 @@ int drm_mode_create_tv_margin_properties(struct drm_device *dev) EXPORT_SYMBOL(drm_mode_create_tv_margin_properties); /** - * drm_mode_create_tv_properties - create TV specific connector properties + * drm_mode_create_tv_properties_legacy - create TV specific connector properties * @dev: DRM device * @num_modes: number of different TV formats (modes) supported * @modes: array of pointers to strings containing name of each format @@ -1652,9 +1652,9 @@ EXPORT_SYMBOL(drm_mode_create_tv_margin_properties); * Returns: * 0 on success or a negative error code on failure. */ -int drm_mode_create_tv_properties(struct drm_device *dev, - unsigned int num_modes, - const char * const modes[]) +int drm_mode_create_tv_properties_legacy(struct drm_device *dev, + unsigned int num_modes, + const char * const modes[]) { struct drm_property *tv_selector; struct drm_property *tv_subconnector; @@ -1737,7 +1737,7 @@ int drm_mode_create_tv_properties(struct drm_device *dev, nomem: return -ENOMEM; } -EXPORT_SYMBOL(drm_mode_create_tv_properties); +EXPORT_SYMBOL(drm_mode_create_tv_properties_legacy); /** * drm_mode_create_scaling_mode_property - create scaling mode property diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c index 86e992b2108b..034e78360d4f 100644 --- a/drivers/gpu/drm/gud/gud_connector.c +++ b/drivers/gpu/drm/gud/gud_connector.c @@ -400,7 +400,7 @@ static int gud_connector_add_tv_mode(struct gud_device *gdrm, struct drm_connect for (i = 0; i < num_modes; i++) modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN]; - ret = drm_mode_create_tv_properties(connector->dev, num_modes, modes); + ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes); free: kfree(buf); if (ret < 0) @@ -539,7 +539,7 @@ static int gud_connector_add_properties(struct gud_device *gdrm, struct gud_conn fallthrough; case GUD_PROPERTY_TV_HUE: /* This is a no-op if already added. */ - ret = drm_mode_create_tv_properties(drm, 0, NULL); + ret = drm_mode_create_tv_properties_legacy(drm, 0, NULL); if (ret) goto out; break; diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c index 9aff24e8e3b2..b9788218b2ec 100644 --- a/drivers/gpu/drm/i2c/ch7006_drv.c +++ b/drivers/gpu/drm/i2c/ch7006_drv.c @@ -250,7 +250,7 @@ static int ch7006_encoder_create_resources(struct drm_encoder *encoder, struct drm_device *dev = encoder->dev; struct drm_mode_config *conf = &dev->mode_config; - drm_mode_create_tv_properties(dev, NUM_TV_NORMS, ch7006_tv_norm_names); + drm_mode_create_tv_properties_legacy(dev, NUM_TV_NORMS, ch7006_tv_norm_names); priv->scale_property = drm_property_create_range(dev, 0, "scale", 0, 2); if (!priv->scale_property) diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c index 5a0483d173d8..b986bf075889 100644 --- a/drivers/gpu/drm/i915/display/intel_tv.c +++ b/drivers/gpu/drm/i915/display/intel_tv.c @@ -1905,7 +1905,7 @@ static void intel_tv_add_properties(struct drm_connector *connector) tv_format_names[i] = tv_modes[i].name; } - drm_mode_create_tv_properties(&i915->drm, i, tv_format_names); + drm_mode_create_tv_properties_legacy(&i915->drm, i, tv_format_names); drm_object_attach_property(&connector->base, i915->drm.mode_config.legacy_tv_mode_property, diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c index 1a15534adc60..e5480dab55e3 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c @@ -653,7 +653,7 @@ static int nv17_tv_create_resources(struct drm_encoder *encoder, tv_enc->tv_norm = i; } - drm_mode_create_tv_properties(dev, num_tv_norms, nv17_tv_norm_names); + drm_mode_create_tv_properties_legacy(dev, num_tv_norms, nv17_tv_norm_names); drm_object_attach_property(&connector->base, conf->tv_select_subconnector_property, diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index e6043cf5d40e..adc9bf99e3fd 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -514,8 +514,9 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data) struct vc4_vec *vec; int ret; - ret = drm_mode_create_tv_properties(drm, ARRAY_SIZE(tv_mode_names), - tv_mode_names); + ret = drm_mode_create_tv_properties_legacy(drm, + ARRAY_SIZE(tv_mode_names), + tv_mode_names); if (ret) return ret; diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 16e47201aaeb..572bd6487247 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1820,9 +1820,9 @@ int drm_mode_create_dvi_i_properties(struct drm_device *dev); void drm_connector_attach_dp_subconnector_property(struct drm_connector *connector); int drm_mode_create_tv_margin_properties(struct drm_device *dev); -int drm_mode_create_tv_properties(struct drm_device *dev, - unsigned int num_modes, - const char * const modes[]); +int drm_mode_create_tv_properties_legacy(struct drm_device *dev, + unsigned int num_modes, + const char * const modes[]); void drm_connector_attach_tv_margin_properties(struct drm_connector *conn); int drm_mode_create_scaling_mode_property(struct drm_device *dev); int drm_connector_attach_content_type_property(struct drm_connector *dev); -- cgit v1.2.3 From c104b23147c725dcdf27558c5f37258aa66e38ed Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 17 Nov 2022 10:28:58 +0100 Subject: drm/vc4: vec: Use TV Reset implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The analog TV properties created by the drm_mode_create_tv_properties() are not properly initialised at reset. Let's switch our implementation to call drm_atomic_helper_connector_tv_reset(). Reviewed-by: Noralf Trønnes Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-15-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index adc9bf99e3fd..90e375a8a8f9 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -268,6 +268,12 @@ vc4_vec_connector_detect(struct drm_connector *connector, bool force) return connector_status_unknown; } +static void vc4_vec_connector_reset(struct drm_connector *connector) +{ + drm_atomic_helper_connector_reset(connector); + drm_atomic_helper_connector_tv_reset(connector); +} + static int vc4_vec_connector_get_modes(struct drm_connector *connector) { struct drm_connector_state *state = connector->state; @@ -288,7 +294,7 @@ static int vc4_vec_connector_get_modes(struct drm_connector *connector) static const struct drm_connector_funcs vc4_vec_connector_funcs = { .detect = vc4_vec_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, - .reset = drm_atomic_helper_connector_reset, + .reset = vc4_vec_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -- cgit v1.2.3 From b5da40af1b89019ffe7ee34f49676514c45411b3 Mon Sep 17 00:00:00 2001 From: Mateusz Kwiatkowski Date: Thu, 17 Nov 2022 10:28:59 +0100 Subject: drm/vc4: vec: Check for VEC output constraints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The VEC can accept pretty much any relatively reasonable mode, but still has a bunch of constraints to meet. Let's create an atomic_check() implementation that will make sure we don't end up accepting a non-functional mode. Acked-by: Noralf Trønnes Signed-off-by: Mateusz Kwiatkowski Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-16-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 50 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index 90e375a8a8f9..bfa8a58dba30 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -453,6 +453,7 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; const struct vc4_vec_tv_mode *vec_mode; vec_mode = &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; @@ -461,6 +462,55 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode)) return -EINVAL; + if (mode->crtc_hdisplay % 4) + return -EINVAL; + + if (!(mode->crtc_hsync_end - mode->crtc_hsync_start)) + return -EINVAL; + + switch (mode->htotal) { + /* NTSC */ + case 858: + if (mode->crtc_vtotal > 262) + return -EINVAL; + + if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 253) + return -EINVAL; + + if (!(mode->crtc_vsync_start - mode->crtc_vdisplay)) + return -EINVAL; + + if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3) + return -EINVAL; + + if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 4) + return -EINVAL; + + break; + + /* PAL/SECAM */ + case 864: + if (mode->crtc_vtotal > 312) + return -EINVAL; + + if (mode->crtc_vdisplay < 1 || mode->crtc_vdisplay > 305) + return -EINVAL; + + if (!(mode->crtc_vsync_start - mode->crtc_vdisplay)) + return -EINVAL; + + if ((mode->crtc_vsync_end - mode->crtc_vsync_start) != 3) + return -EINVAL; + + if ((mode->crtc_vtotal - mode->crtc_vsync_end) < 2) + return -EINVAL; + + break; + + default: + return -EINVAL; + } + return 0; } -- cgit v1.2.3 From 91112a6fc12cb086cf7a3a350df6a415ba4631ef Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 17 Nov 2022 10:29:00 +0100 Subject: drm/vc4: vec: Convert to the new TV mode property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the core can deal fine with analog TV modes, let's convert the vc4 VEC driver to leverage those new features. We've added some backward compatibility to support the old TV mode property and translate it into the new TV norm property. We're also making use of the new analog TV atomic_check helper to make sure we trigger a modeset whenever the TV mode is updated. Acked-by: Noralf Trønnes Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-17-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 184 ++++++++++++++++++++++++++++++------------ 1 file changed, 131 insertions(+), 53 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index bfa8a58dba30..a828fc6fb776 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -172,6 +172,8 @@ struct vc4_vec { struct clk *clock; + struct drm_property *legacy_tv_mode_property; + struct debugfs_regset32 regset; }; @@ -184,6 +186,12 @@ encoder_to_vc4_vec(struct drm_encoder *encoder) return container_of(encoder, struct vc4_vec, encoder.base); } +static inline struct vc4_vec * +connector_to_vc4_vec(struct drm_connector *connector) +{ + return container_of(connector, struct vc4_vec, connector); +} + enum vc4_vec_tv_mode_id { VC4_VEC_TV_MODE_NTSC, VC4_VEC_TV_MODE_NTSC_J, @@ -192,7 +200,7 @@ enum vc4_vec_tv_mode_id { }; struct vc4_vec_tv_mode { - const struct drm_display_mode *mode; + unsigned int mode; u32 config0; u32 config1; u32 custom_freq; @@ -225,43 +233,51 @@ static const struct debugfs_reg32 vec_regs[] = { VC4_REG32(VEC_DAC_MISC), }; -static const struct drm_display_mode ntsc_mode = { - DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500, - 720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0, - 480, 480 + 7, 480 + 7 + 6, 525, 0, - DRM_MODE_FLAG_INTERLACE) -}; - -static const struct drm_display_mode pal_mode = { - DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500, - 720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0, - 576, 576 + 4, 576 + 4 + 6, 625, 0, - DRM_MODE_FLAG_INTERLACE) -}; - static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { - [VC4_VEC_TV_MODE_NTSC] = { - .mode = &ntsc_mode, + { + .mode = DRM_MODE_TV_MODE_NTSC, .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, - [VC4_VEC_TV_MODE_NTSC_J] = { - .mode = &ntsc_mode, + { + .mode = DRM_MODE_TV_MODE_NTSC_J, .config0 = VEC_CONFIG0_NTSC_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, - [VC4_VEC_TV_MODE_PAL] = { - .mode = &pal_mode, + { + .mode = DRM_MODE_TV_MODE_PAL, .config0 = VEC_CONFIG0_PAL_BDGHI_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, - [VC4_VEC_TV_MODE_PAL_M] = { - .mode = &ntsc_mode, + { + .mode = DRM_MODE_TV_MODE_PAL_M, .config0 = VEC_CONFIG0_PAL_M_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, }; +static inline const struct vc4_vec_tv_mode * +vc4_vec_tv_mode_lookup(unsigned int mode) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) { + const struct vc4_vec_tv_mode *tv_mode = &vc4_vec_tv_modes[i]; + + if (tv_mode->mode == mode) + return tv_mode; + } + + return NULL; +} + +static const struct drm_prop_enum_list legacy_tv_mode_names[] = { + { VC4_VEC_TV_MODE_NTSC, "NTSC", }, + { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", }, + { VC4_VEC_TV_MODE_PAL, "PAL", }, + { VC4_VEC_TV_MODE_PAL_M, "PAL-M", }, +}; + static enum drm_connector_status vc4_vec_connector_detect(struct drm_connector *connector, bool force) { @@ -274,21 +290,74 @@ static void vc4_vec_connector_reset(struct drm_connector *connector) drm_atomic_helper_connector_tv_reset(connector); } -static int vc4_vec_connector_get_modes(struct drm_connector *connector) +static int +vc4_vec_connector_set_property(struct drm_connector *connector, + struct drm_connector_state *state, + struct drm_property *property, + uint64_t val) { - struct drm_connector_state *state = connector->state; - struct drm_display_mode *mode; + struct vc4_vec *vec = connector_to_vc4_vec(connector); - mode = drm_mode_duplicate(connector->dev, - vc4_vec_tv_modes[state->tv.legacy_mode].mode); - if (!mode) { - DRM_ERROR("Failed to create a new display mode\n"); - return -ENOMEM; + if (property != vec->legacy_tv_mode_property) + return -EINVAL; + + switch (val) { + case VC4_VEC_TV_MODE_NTSC: + state->tv.mode = DRM_MODE_TV_MODE_NTSC; + break; + + case VC4_VEC_TV_MODE_NTSC_J: + state->tv.mode = DRM_MODE_TV_MODE_NTSC_J; + break; + + case VC4_VEC_TV_MODE_PAL: + state->tv.mode = DRM_MODE_TV_MODE_PAL; + break; + + case VC4_VEC_TV_MODE_PAL_M: + state->tv.mode = DRM_MODE_TV_MODE_PAL_M; + break; + + default: + return -EINVAL; } - drm_mode_probed_add(connector, mode); + return 0; +} + +static int +vc4_vec_connector_get_property(struct drm_connector *connector, + const struct drm_connector_state *state, + struct drm_property *property, + uint64_t *val) +{ + struct vc4_vec *vec = connector_to_vc4_vec(connector); + + if (property != vec->legacy_tv_mode_property) + return -EINVAL; - return 1; + switch (state->tv.mode) { + case DRM_MODE_TV_MODE_NTSC: + *val = VC4_VEC_TV_MODE_NTSC; + break; + + case DRM_MODE_TV_MODE_NTSC_J: + *val = VC4_VEC_TV_MODE_NTSC_J; + break; + + case DRM_MODE_TV_MODE_PAL: + *val = VC4_VEC_TV_MODE_PAL; + break; + + case DRM_MODE_TV_MODE_PAL_M: + *val = VC4_VEC_TV_MODE_PAL_M; + break; + + default: + return -EINVAL; + } + + return 0; } static const struct drm_connector_funcs vc4_vec_connector_funcs = { @@ -297,15 +366,19 @@ static const struct drm_connector_funcs vc4_vec_connector_funcs = { .reset = vc4_vec_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, + .atomic_get_property = vc4_vec_connector_get_property, + .atomic_set_property = vc4_vec_connector_set_property, }; static const struct drm_connector_helper_funcs vc4_vec_connector_helper_funcs = { - .get_modes = vc4_vec_connector_get_modes, + .atomic_check = drm_atomic_helper_connector_tv_check, + .get_modes = drm_connector_helper_tv_get_modes, }; static int vc4_vec_connector_init(struct drm_device *dev, struct vc4_vec *vec) { struct drm_connector *connector = &vec->connector; + struct drm_property *prop; int ret; connector->interlace_allowed = true; @@ -318,8 +391,17 @@ static int vc4_vec_connector_init(struct drm_device *dev, struct vc4_vec *vec) drm_connector_helper_add(connector, &vc4_vec_connector_helper_funcs); drm_object_attach_property(&connector->base, - dev->mode_config.legacy_tv_mode_property, - VC4_VEC_TV_MODE_NTSC); + dev->mode_config.tv_mode_property, + DRM_MODE_TV_MODE_NTSC); + + prop = drm_property_create_enum(dev, 0, "mode", + legacy_tv_mode_names, + ARRAY_SIZE(legacy_tv_mode_names)); + if (!prop) + return -ENOMEM; + vec->legacy_tv_mode_property = prop; + + drm_object_attach_property(&connector->base, prop, VC4_VEC_TV_MODE_NTSC); drm_connector_attach_encoder(connector, &vec->encoder.base); @@ -366,13 +448,16 @@ static void vc4_vec_encoder_enable(struct drm_encoder *encoder, struct drm_connector *connector = &vec->connector; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); - const struct vc4_vec_tv_mode *tv_mode = - &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; + const struct vc4_vec_tv_mode *tv_mode; int idx, ret; if (!drm_dev_enter(drm, &idx)) return; + tv_mode = vc4_vec_tv_mode_lookup(conn_state->tv.mode); + if (!tv_mode) + goto err_dev_exit; + ret = pm_runtime_get_sync(&vec->pdev->dev); if (ret < 0) { DRM_ERROR("Failed to retain power domain: %d\n", ret); @@ -454,12 +539,10 @@ static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder, struct drm_connector_state *conn_state) { const struct drm_display_mode *mode = &crtc_state->adjusted_mode; - const struct vc4_vec_tv_mode *vec_mode; + const struct vc4_vec_tv_mode *tv_mode; - vec_mode = &vc4_vec_tv_modes[conn_state->tv.legacy_mode]; - - if (conn_state->crtc && - !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode)) + tv_mode = vc4_vec_tv_mode_lookup(conn_state->tv.mode, mode->htotal); + if (!tv_mode) return -EINVAL; if (mode->crtc_hdisplay % 4) @@ -556,13 +639,6 @@ static const struct of_device_id vc4_vec_dt_match[] = { { /* sentinel */ }, }; -static const char * const tv_mode_names[] = { - [VC4_VEC_TV_MODE_NTSC] = "NTSC", - [VC4_VEC_TV_MODE_NTSC_J] = "NTSC-J", - [VC4_VEC_TV_MODE_PAL] = "PAL", - [VC4_VEC_TV_MODE_PAL_M] = "PAL-M", -}; - static int vc4_vec_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); @@ -570,9 +646,11 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data) struct vc4_vec *vec; int ret; - ret = drm_mode_create_tv_properties_legacy(drm, - ARRAY_SIZE(tv_mode_names), - tv_mode_names); + ret = drm_mode_create_tv_properties(drm, + BIT(DRM_MODE_TV_MODE_NTSC) | + BIT(DRM_MODE_TV_MODE_NTSC_J) | + BIT(DRM_MODE_TV_MODE_PAL) | + BIT(DRM_MODE_TV_MODE_PAL_M)); if (ret) return ret; -- cgit v1.2.3 From 2757279304e45d749578df30ae0cd1a42fefe096 Mon Sep 17 00:00:00 2001 From: Mateusz Kwiatkowski Date: Thu, 17 Nov 2022 10:29:01 +0100 Subject: drm/vc4: vec: Add support for more analog TV standards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for the following composite output modes (all of them are somewhat more obscure than the previously defined ones): - NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to 4.43361875 MHz (the PAL subcarrier frequency). Never used for broadcasting, but sometimes used as a hack to play NTSC content in PAL regions (e.g. on VCRs). - PAL_N - PAL with alternative chroma subcarrier frequency, 3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay and Uruguay to fit 576i50 with colour in 6 MHz channel raster. - PAL60 - 480i60 signal with PAL-style color at normal European PAL frequency. Another non-standard, non-broadcast mode, used in similar contexts as NTSC_443. Some displays support one but not the other. - SECAM - French frequency-modulated analog color standard; also have been broadcast in Eastern Europe and various parts of Africa and Asia. Uses the same 576i50 timings as PAL. Also added some comments explaining color subcarrier frequency registers. Acked-by: Noralf Trønnes Signed-off-by: Mateusz Kwiatkowski Tested-by: Mateusz Kwiatkowski Acked-in-principle-or-something-like-that-by: Daniel Vetter Link: https://lore.kernel.org/r/20220728-rpi-analog-tv-properties-v10-18-256dad125326@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 111 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index a828fc6fb776..d23dbad3cbf6 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -46,6 +46,7 @@ #define VEC_CONFIG0_YDEL(x) ((x) << 26) #define VEC_CONFIG0_CDEL_MASK GENMASK(25, 24) #define VEC_CONFIG0_CDEL(x) ((x) << 24) +#define VEC_CONFIG0_SECAM_STD BIT(21) #define VEC_CONFIG0_PBPR_FIL BIT(18) #define VEC_CONFIG0_CHROMA_GAIN_MASK GENMASK(17, 16) #define VEC_CONFIG0_CHROMA_GAIN_UNITY (0 << 16) @@ -76,6 +77,27 @@ #define VEC_SOFT_RESET 0x10c #define VEC_CLMP0_START 0x144 #define VEC_CLMP0_END 0x148 + +/* + * These set the color subcarrier frequency + * if VEC_CONFIG1_CUSTOM_FREQ is enabled. + * + * VEC_FREQ1_0 contains the most significant 16-bit half-word, + * VEC_FREQ3_2 contains the least significant 16-bit half-word. + * 0x80000000 seems to be equivalent to the pixel clock + * (which itself is the VEC clock divided by 8). + * + * Reference values (with the default pixel clock of 13.5 MHz): + * + * NTSC (3579545.[45] Hz) - 0x21F07C1F + * PAL (4433618.75 Hz) - 0x2A098ACB + * PAL-M (3575611.[888111] Hz) - 0x21E6EFE3 + * PAL-N (3582056.25 Hz) - 0x21F69446 + * + * NOTE: For SECAM, it is used as the Dr center frequency, + * regardless of whether VEC_CONFIG1_CUSTOM_FREQ is enabled or not; + * that is specified as 4406250 Hz, which corresponds to 0x29C71C72. + */ #define VEC_FREQ3_2 0x180 #define VEC_FREQ1_0 0x184 @@ -118,6 +140,14 @@ #define VEC_INTERRUPT_CONTROL 0x190 #define VEC_INTERRUPT_STATUS 0x194 + +/* + * Db center frequency for SECAM; the clock for this is the same as for + * VEC_FREQ3_2/VEC_FREQ1_0, which is used for Dr center frequency. + * + * This is specified as 4250000 Hz, which corresponds to 0x284BDA13. + * That is also the default value, so no need to set it explicitly. + */ #define VEC_FCW_SECAM_B 0x198 #define VEC_SECAM_GAIN_VAL 0x19c @@ -197,10 +227,15 @@ enum vc4_vec_tv_mode_id { VC4_VEC_TV_MODE_NTSC_J, VC4_VEC_TV_MODE_PAL, VC4_VEC_TV_MODE_PAL_M, + VC4_VEC_TV_MODE_NTSC_443, + VC4_VEC_TV_MODE_PAL_60, + VC4_VEC_TV_MODE_PAL_N, + VC4_VEC_TV_MODE_SECAM, }; struct vc4_vec_tv_mode { unsigned int mode; + u16 expected_htotal; u32 config0; u32 config1; u32 custom_freq; @@ -236,35 +271,68 @@ static const struct debugfs_reg32 vec_regs[] = { static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = { { .mode = DRM_MODE_TV_MODE_NTSC, + .expected_htotal = 858, .config0 = VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, + { + .mode = DRM_MODE_TV_MODE_NTSC_443, + .expected_htotal = 858, + .config0 = VEC_CONFIG0_NTSC_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, + .custom_freq = 0x2a098acb, + }, { .mode = DRM_MODE_TV_MODE_NTSC_J, + .expected_htotal = 858, .config0 = VEC_CONFIG0_NTSC_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, { .mode = DRM_MODE_TV_MODE_PAL, + .expected_htotal = 864, .config0 = VEC_CONFIG0_PAL_BDGHI_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, + { + /* PAL-60 */ + .mode = DRM_MODE_TV_MODE_PAL, + .expected_htotal = 858, + .config0 = VEC_CONFIG0_PAL_M_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ, + .custom_freq = 0x2a098acb, + }, { .mode = DRM_MODE_TV_MODE_PAL_M, + .expected_htotal = 858, .config0 = VEC_CONFIG0_PAL_M_STD, .config1 = VEC_CONFIG1_C_CVBS_CVBS, }, + { + .mode = DRM_MODE_TV_MODE_PAL_N, + .expected_htotal = 864, + .config0 = VEC_CONFIG0_PAL_N_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS, + }, + { + .mode = DRM_MODE_TV_MODE_SECAM, + .expected_htotal = 864, + .config0 = VEC_CONFIG0_SECAM_STD, + .config1 = VEC_CONFIG1_C_CVBS_CVBS, + .custom_freq = 0x29c71c72, + }, }; static inline const struct vc4_vec_tv_mode * -vc4_vec_tv_mode_lookup(unsigned int mode) +vc4_vec_tv_mode_lookup(unsigned int mode, u16 htotal) { unsigned int i; for (i = 0; i < ARRAY_SIZE(vc4_vec_tv_modes); i++) { const struct vc4_vec_tv_mode *tv_mode = &vc4_vec_tv_modes[i]; - if (tv_mode->mode == mode) + if (tv_mode->mode == mode && + tv_mode->expected_htotal == htotal) return tv_mode; } @@ -273,9 +341,13 @@ vc4_vec_tv_mode_lookup(unsigned int mode) static const struct drm_prop_enum_list legacy_tv_mode_names[] = { { VC4_VEC_TV_MODE_NTSC, "NTSC", }, + { VC4_VEC_TV_MODE_NTSC_443, "NTSC-443", }, { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", }, { VC4_VEC_TV_MODE_PAL, "PAL", }, + { VC4_VEC_TV_MODE_PAL_60, "PAL-60", }, { VC4_VEC_TV_MODE_PAL_M, "PAL-M", }, + { VC4_VEC_TV_MODE_PAL_N, "PAL-N", }, + { VC4_VEC_TV_MODE_SECAM, "SECAM", }, }; static enum drm_connector_status @@ -306,11 +378,16 @@ vc4_vec_connector_set_property(struct drm_connector *connector, state->tv.mode = DRM_MODE_TV_MODE_NTSC; break; + case VC4_VEC_TV_MODE_NTSC_443: + state->tv.mode = DRM_MODE_TV_MODE_NTSC_443; + break; + case VC4_VEC_TV_MODE_NTSC_J: state->tv.mode = DRM_MODE_TV_MODE_NTSC_J; break; case VC4_VEC_TV_MODE_PAL: + case VC4_VEC_TV_MODE_PAL_60: state->tv.mode = DRM_MODE_TV_MODE_PAL; break; @@ -318,6 +395,14 @@ vc4_vec_connector_set_property(struct drm_connector *connector, state->tv.mode = DRM_MODE_TV_MODE_PAL_M; break; + case VC4_VEC_TV_MODE_PAL_N: + state->tv.mode = DRM_MODE_TV_MODE_PAL_N; + break; + + case VC4_VEC_TV_MODE_SECAM: + state->tv.mode = DRM_MODE_TV_MODE_SECAM; + break; + default: return -EINVAL; } @@ -341,6 +426,10 @@ vc4_vec_connector_get_property(struct drm_connector *connector, *val = VC4_VEC_TV_MODE_NTSC; break; + case DRM_MODE_TV_MODE_NTSC_443: + *val = VC4_VEC_TV_MODE_NTSC_443; + break; + case DRM_MODE_TV_MODE_NTSC_J: *val = VC4_VEC_TV_MODE_NTSC_J; break; @@ -353,6 +442,14 @@ vc4_vec_connector_get_property(struct drm_connector *connector, *val = VC4_VEC_TV_MODE_PAL_M; break; + case DRM_MODE_TV_MODE_PAL_N: + *val = VC4_VEC_TV_MODE_PAL_N; + break; + + case DRM_MODE_TV_MODE_SECAM: + *val = VC4_VEC_TV_MODE_SECAM; + break; + default: return -EINVAL; } @@ -448,13 +545,16 @@ static void vc4_vec_encoder_enable(struct drm_encoder *encoder, struct drm_connector *connector = &vec->connector; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); + struct drm_display_mode *adjusted_mode = + &encoder->crtc->state->adjusted_mode; const struct vc4_vec_tv_mode *tv_mode; int idx, ret; if (!drm_dev_enter(drm, &idx)) return; - tv_mode = vc4_vec_tv_mode_lookup(conn_state->tv.mode); + tv_mode = vc4_vec_tv_mode_lookup(conn_state->tv.mode, + adjusted_mode->htotal); if (!tv_mode) goto err_dev_exit; @@ -648,9 +748,12 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data) ret = drm_mode_create_tv_properties(drm, BIT(DRM_MODE_TV_MODE_NTSC) | + BIT(DRM_MODE_TV_MODE_NTSC_443) | BIT(DRM_MODE_TV_MODE_NTSC_J) | BIT(DRM_MODE_TV_MODE_PAL) | - BIT(DRM_MODE_TV_MODE_PAL_M)); + BIT(DRM_MODE_TV_MODE_PAL_M) | + BIT(DRM_MODE_TV_MODE_PAL_N) | + BIT(DRM_MODE_TV_MODE_SECAM)); if (ret) return ret; -- cgit v1.2.3 From 0ae41323a83233610e64e926eefb4d132ecb9028 Mon Sep 17 00:00:00 2001 From: Shang XiaoJing Date: Thu, 24 Nov 2022 09:51:13 +0800 Subject: drm/vc4: vec: Use pm_runtime_resume_and_get() in vc4_vec_encoder_enable() Commit f0601ef8631c ("drm/vc4: vec: Protect device resources after removal") add fail path for vc4_vec_encoder_enable(), and will put usage_counter only when pm_runtime_get_sync() succeeds. However, pm_runtime_get_sync() will increment usage_counter even it failed. Fix it by replacing it with pm_runtime_resume_and_get() to keep usage counter balanced. Fixes: e4b81f8c74c8 ("drm/vc4: Add support for the VEC (Video Encoder) IP") Signed-off-by: Shang XiaoJing Link: https://lore.kernel.org/r/20221124015113.18540-1-shangxiaojing@huawei.com Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_vec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index d23dbad3cbf6..e270a4099be3 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -558,7 +558,7 @@ static void vc4_vec_encoder_enable(struct drm_encoder *encoder, if (!tv_mode) goto err_dev_exit; - ret = pm_runtime_get_sync(&vec->pdev->dev); + ret = pm_runtime_resume_and_get(&vec->pdev->dev); if (ret < 0) { DRM_ERROR("Failed to retain power domain: %d\n", ret); goto err_dev_exit; -- cgit v1.2.3 From da43ff045c3fc4c11e4956cf42c59aba7e5ed39e Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 1 Dec 2022 16:11:49 +0100 Subject: drm/vc4: tests: Fail the current test if we access a register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accessing a register when running under kunit is a bad idea since our device is completely mocked. Fail the current test if we ever access any of our hardware registers. Reviewed-by: Javier Martinez Canillas Reviewed-by: Maíra Canal Link: https://lore.kernel.org/r/20221123-rpi-kunit-tests-v3-18-4615a663a84a@cerno.tech Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_crtc.c | 13 +++++++++++-- drivers/gpu/drm/vc4/vc4_dpi.c | 13 +++++++++++-- drivers/gpu/drm/vc4/vc4_drv.h | 29 +++++++++++++++++++++++++---- drivers/gpu/drm/vc4/vc4_dsi.c | 9 ++++++++- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 ++++ drivers/gpu/drm/vc4/vc4_txp.c | 13 +++++++++++-- drivers/gpu/drm/vc4/vc4_vec.c | 13 +++++++++++-- 7 files changed, 81 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 7d1a696477ce..a1a3465948c4 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -50,8 +50,17 @@ #define HVS_FIFO_LATENCY_PIX 6 -#define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset)) -#define CRTC_READ(offset) readl(vc4_crtc->regs + (offset)) +#define CRTC_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, vc4_crtc->regs + (offset)); \ + } while (0) + +#define CRTC_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(vc4_crtc->regs + (offset)); \ + }) static const struct debugfs_reg32 crtc_regs[] = { VC4_REG32(PV_CONTROL), diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index a7bebfa5d5b0..d31a02af1e53 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -103,8 +103,17 @@ to_vc4_dpi(struct drm_encoder *encoder) return container_of(encoder, struct vc4_dpi, encoder.base); } -#define DPI_READ(offset) readl(dpi->regs + (offset)) -#define DPI_WRITE(offset, val) writel(val, dpi->regs + (offset)) +#define DPI_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(dpi->regs + (offset)); \ + }) + +#define DPI_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, dpi->regs + (offset)); \ + } while (0) static const struct debugfs_reg32 dpi_regs[] = { VC4_REG32(DPI_C), diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 418f4f308e36..78fda5332cb3 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -19,6 +19,8 @@ #include #include +#include + #include "uapi/drm/vc4_drm.h" struct drm_device; @@ -645,10 +647,29 @@ to_vc4_crtc_state(const struct drm_crtc_state *crtc_state) return container_of(crtc_state, struct vc4_crtc_state, base); } -#define V3D_READ(offset) readl(vc4->v3d->regs + offset) -#define V3D_WRITE(offset, val) writel(val, vc4->v3d->regs + offset) -#define HVS_READ(offset) readl(hvs->regs + offset) -#define HVS_WRITE(offset, val) writel(val, hvs->regs + offset) +#define V3D_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(vc4->v3d->regs + (offset)); \ + }) + +#define V3D_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, vc4->v3d->regs + (offset)); \ + } while (0) + +#define HVS_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(hvs->regs + (offset)); \ + }) + +#define HVS_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, hvs->regs + (offset)); \ + } while (0) #define VC4_REG32(reg) { .name = #reg, .offset = reg } diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 878e05d79e81..2c9cb27903a0 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -617,6 +617,8 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val) dma_cookie_t cookie; int ret; + kunit_fail_current_test("Accessing a register in a unit test!\n"); + /* DSI0 should be able to write normally. */ if (!chan) { writel(val, dsi->regs + offset); @@ -645,7 +647,12 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val) DRM_ERROR("Failed to wait for DMA: %d\n", ret); } -#define DSI_READ(offset) readl(dsi->regs + (offset)) +#define DSI_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(dsi->regs + (offset)); \ + }) + #define DSI_WRITE(offset, val) dsi_dma_workaround_write(dsi, offset, val) #define DSI_PORT_READ(offset) \ DSI_READ(dsi->variant->port ? DSI1_##offset : DSI0_##offset) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index 48db438550b1..b04b2fc8d831 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -456,6 +456,8 @@ static inline u32 vc4_hdmi_read(struct vc4_hdmi *hdmi, WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev)); + kunit_fail_current_test("Accessing an HDMI register in a unit test!\n"); + if (reg >= variant->num_registers) { dev_warn(&hdmi->pdev->dev, "Invalid register ID %u\n", reg); @@ -486,6 +488,8 @@ static inline void vc4_hdmi_write(struct vc4_hdmi *hdmi, WARN_ON(pm_runtime_status_suspended(&hdmi->pdev->dev)); + kunit_fail_current_test("Accessing an HDMI register in a unit test!\n"); + if (reg >= variant->num_registers) { dev_warn(&hdmi->pdev->dev, "Invalid register ID %u\n", reg); diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 2b69454b8534..ef5cab2a3aa9 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -145,8 +145,17 @@ /* Number of lines received and committed to memory. */ #define TXP_PROGRESS 0x10 -#define TXP_READ(offset) readl(txp->regs + (offset)) -#define TXP_WRITE(offset, val) writel(val, txp->regs + (offset)) +#define TXP_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(txp->regs + (offset)); \ + }) + +#define TXP_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, txp->regs + (offset)); \ + } while (0) struct vc4_txp { struct vc4_crtc base; diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index e270a4099be3..f589ab918f4d 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -207,8 +207,17 @@ struct vc4_vec { struct debugfs_regset32 regset; }; -#define VEC_READ(offset) readl(vec->regs + (offset)) -#define VEC_WRITE(offset, val) writel(val, vec->regs + (offset)) +#define VEC_READ(offset) \ + ({ \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + readl(vec->regs + (offset)); \ + }) + +#define VEC_WRITE(offset, val) \ + do { \ + kunit_fail_current_test("Accessing a register in a unit test!\n"); \ + writel(val, vec->regs + (offset)); \ + } while (0) static inline struct vc4_vec * encoder_to_vc4_vec(struct drm_encoder *encoder) -- cgit v1.2.3 From f2ede40e46e8cc64fcdac36c6942b42b21ef74ec Mon Sep 17 00:00:00 2001 From: Maíra Canal Date: Mon, 19 Dec 2022 09:06:18 -0300 Subject: drm/vc4: use new debugfs device-centered functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, vc4 has its own debugfs infrastructure that adds the debugfs files on drm_dev_register(). With the introduction of the new debugfs, functions, replace the vc4 debugfs structure with the DRM debugfs device-centered function, drm_debugfs_add_file(). Moreover, remove the explicit error handling of debugfs related functions, considering that the only failure mode is -ENOMEM and also that error handling is not recommended for debugfs functions, as pointed out in [1]. [1] https://lore.kernel.org/all/YWAmZdRwnAt6wh9B@kroah.com/ Signed-off-by: Maíra Canal Reviewed-by: Maxime Ripard Reviewed-by: Melissa Wen Acked-by: Daniel Vetter Signed-off-by: Maíra Canal Link: https://patchwork.freedesktop.org/patch/msgid/20221219120621.15086-5-mcanal@igalia.com --- drivers/gpu/drm/vc4/vc4_bo.c | 10 +++------- drivers/gpu/drm/vc4/vc4_crtc.c | 7 ++----- drivers/gpu/drm/vc4/vc4_debugfs.c | 36 +++++++----------------------------- drivers/gpu/drm/vc4/vc4_dpi.c | 5 +---- drivers/gpu/drm/vc4/vc4_drv.c | 1 - drivers/gpu/drm/vc4/vc4_drv.h | 32 +++++++------------------------- drivers/gpu/drm/vc4/vc4_dsi.c | 6 +----- drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++-------- drivers/gpu/drm/vc4/vc4_hvs.c | 24 +++++++----------------- drivers/gpu/drm/vc4/vc4_v3d.c | 14 ++++---------- drivers/gpu/drm/vc4/vc4_vec.c | 6 +----- 11 files changed, 37 insertions(+), 116 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_vec.c') diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 43d9b3a6a352..c2b7573bd92b 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -69,8 +69,8 @@ static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4) static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused) { - struct drm_info_node *node = (struct drm_info_node *)m->private; - struct drm_device *dev = node->minor->dev; + struct drm_debugfs_entry *entry = m->private; + struct drm_device *dev = entry->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_printer p = drm_seq_file_printer(m); @@ -993,15 +993,11 @@ int vc4_bo_debugfs_init(struct drm_minor *minor) { struct drm_device *drm = minor->dev; struct vc4_dev *vc4 = to_vc4_dev(drm); - int ret; if (!vc4->v3d) return -ENODEV; - ret = vc4_debugfs_add_file(minor, "bo_stats", - vc4_bo_stats_debugfs, NULL); - if (ret) - return ret; + drm_debugfs_add_file(drm, "bo_stats", vc4_bo_stats_debugfs, NULL); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index a1a3465948c4..35a6b5907278 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1090,12 +1090,9 @@ int vc4_crtc_late_register(struct drm_crtc *crtc) struct drm_device *drm = crtc->dev; struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); const struct vc4_crtc_data *crtc_data = vc4_crtc_to_vc4_crtc_data(vc4_crtc); - int ret; - ret = vc4_debugfs_add_regset32(drm->primary, crtc_data->debugfs_name, - &vc4_crtc->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, crtc_data->debugfs_name, + &vc4_crtc->regset); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c index 19cda4f91a82..fac624a663ea 100644 --- a/drivers/gpu/drm/vc4/vc4_debugfs.c +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c @@ -34,9 +34,9 @@ vc4_debugfs_init(struct drm_minor *minor) static int vc4_debugfs_regset32(struct seq_file *m, void *unused) { - struct drm_info_node *node = (struct drm_info_node *)m->private; - struct drm_device *drm = node->minor->dev; - struct debugfs_regset32 *regset = node->info_ent->data; + struct drm_debugfs_entry *entry = m->private; + struct drm_device *drm = entry->dev; + struct debugfs_regset32 *regset = entry->file.data; struct drm_printer p = drm_seq_file_printer(m); int idx; @@ -50,31 +50,9 @@ static int vc4_debugfs_regset32(struct seq_file *m, void *unused) return 0; } -int vc4_debugfs_add_file(struct drm_minor *minor, - const char *name, - int (*show)(struct seq_file*, void*), - void *data) +void vc4_debugfs_add_regset32(struct drm_device *drm, + const char *name, + struct debugfs_regset32 *regset) { - struct drm_device *dev = minor->dev; - struct dentry *root = minor->debugfs_root; - struct drm_info_list *file; - - file = drmm_kzalloc(dev, sizeof(*file), GFP_KERNEL); - if (!file) - return -ENOMEM; - - file->name = name; - file->show = show; - file->data = data; - - drm_debugfs_create_files(file, 1, root, minor); - - return 0; -} - -int vc4_debugfs_add_regset32(struct drm_minor *minor, - const char *name, - struct debugfs_regset32 *regset) -{ - return vc4_debugfs_add_file(minor, name, vc4_debugfs_regset32, regset); + drm_debugfs_add_file(drm, name, vc4_debugfs_regset32, regset); } diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index d31a02af1e53..f518d6e59ed6 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -267,11 +267,8 @@ static int vc4_dpi_late_register(struct drm_encoder *encoder) { struct drm_device *drm = encoder->dev; struct vc4_dpi *dpi = to_vc4_dpi(encoder); - int ret; - ret = vc4_debugfs_add_regset32(drm->primary, "dpi_regs", &dpi->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 3b0ae2c3e33c..0ccaee57fe9a 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -320,7 +320,6 @@ static int vc4_drm_bind(struct device *dev) drm = &vc4->base; platform_set_drvdata(pdev, drm); - INIT_LIST_HEAD(&vc4->debugfs_list); if (!is_vc5) { ret = drmm_mutex_init(drm, &vc4->bin_bo_lock); diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 78fda5332cb3..95069bb16821 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -226,11 +226,6 @@ struct vc4_dev { struct drm_private_obj hvs_channels; struct drm_private_obj load_tracker; - /* List of vc4_debugfs_info_entry for adding to debugfs once - * the minor is available (after drm_dev_register()). - */ - struct list_head debugfs_list; - /* Mutex for binner bo allocation. */ struct mutex bin_bo_lock; /* Reference count for our binner bo. */ @@ -971,28 +966,15 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state, /* vc4_debugfs.c */ void vc4_debugfs_init(struct drm_minor *minor); #ifdef CONFIG_DEBUG_FS -int vc4_debugfs_add_file(struct drm_minor *minor, - const char *filename, - int (*show)(struct seq_file*, void*), - void *data); -int vc4_debugfs_add_regset32(struct drm_minor *minor, - const char *filename, - struct debugfs_regset32 *regset); +void vc4_debugfs_add_regset32(struct drm_device *drm, + const char *filename, + struct debugfs_regset32 *regset); #else -static inline int vc4_debugfs_add_file(struct drm_minor *minor, - const char *filename, - int (*show)(struct seq_file*, void*), - void *data) -{ - return 0; -} -static inline int vc4_debugfs_add_regset32(struct drm_minor *minor, - const char *filename, - struct debugfs_regset32 *regset) -{ - return 0; -} +static inline void vc4_debugfs_add_regset32(struct drm_device *drm, + const char *filename, + struct debugfs_regset32 *regset) +{} #endif /* vc4_drv.c */ diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 0ee9ada428ab..e05be9a34156 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1427,12 +1427,8 @@ static int vc4_dsi_late_register(struct drm_encoder *encoder) { struct drm_device *drm = encoder->dev; struct vc4_dsi *dsi = to_vc4_dsi(encoder); - int ret; - ret = vc4_debugfs_add_regset32(drm->primary, dsi->variant->debugfs_name, - &dsi->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 12a00d644b61..0d3313c71f2f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -160,8 +160,8 @@ static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi, static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) { - struct drm_info_node *node = (struct drm_info_node *)m->private; - struct vc4_hdmi *vc4_hdmi = node->info_ent->data; + struct drm_debugfs_entry *entry = m->private; + struct vc4_hdmi *vc4_hdmi = entry->file.data; struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_printer p = drm_seq_file_printer(m); int idx; @@ -1995,13 +1995,9 @@ static int vc4_hdmi_late_register(struct drm_encoder *encoder) struct drm_device *drm = encoder->dev; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); const struct vc4_hdmi_variant *variant = vc4_hdmi->variant; - int ret; - ret = vc4_debugfs_add_file(drm->primary, variant->debugfs_name, - vc4_hdmi_debugfs_regs, - vc4_hdmi); - if (ret) - return ret; + drm_debugfs_add_file(drm, variant->debugfs_name, + vc4_hdmi_debugfs_regs, vc4_hdmi); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 94c29f8547bb..024a2cdff5b2 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -93,8 +93,8 @@ void vc4_hvs_dump_state(struct vc4_hvs *hvs) static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data) { - struct drm_info_node *node = m->private; - struct drm_device *dev = node->minor->dev; + struct drm_debugfs_entry *entry = m->private; + struct drm_device *dev = entry->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_printer p = drm_seq_file_printer(m); @@ -105,8 +105,8 @@ static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data) 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 drm_debugfs_entry *entry = m->private; + struct drm_device *dev = entry->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_hvs *hvs = vc4->hvs; struct drm_printer p = drm_seq_file_printer(m); @@ -740,7 +740,6 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor) struct drm_device *drm = minor->dev; struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_hvs *hvs = vc4->hvs; - int ret; if (!vc4->hvs) return -ENODEV; @@ -750,20 +749,11 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor) minor->debugfs_root, &vc4->load_tracker_enabled); - ret = vc4_debugfs_add_file(minor, "hvs_dlists", - vc4_hvs_debugfs_dlist, NULL); - if (ret) - return ret; + drm_debugfs_add_file(drm, "hvs_dlists", vc4_hvs_debugfs_dlist, NULL); - ret = vc4_debugfs_add_file(minor, "hvs_underrun", - vc4_hvs_debugfs_underrun, NULL); - if (ret) - return ret; + drm_debugfs_add_file(drm, "hvs_underrun", vc4_hvs_debugfs_underrun, NULL); - ret = vc4_debugfs_add_regset32(minor, "hvs_regs", - &hvs->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index 56abb0d6bc39..29a664c8bf44 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -96,8 +96,8 @@ static const struct debugfs_reg32 v3d_regs[] = { static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused) { - struct drm_info_node *node = (struct drm_info_node *)m->private; - struct drm_device *dev = node->minor->dev; + struct drm_debugfs_entry *entry = m->private; + struct drm_device *dev = entry->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); int ret = vc4_v3d_pm_get(vc4); @@ -404,19 +404,13 @@ int vc4_v3d_debugfs_init(struct drm_minor *minor) struct drm_device *drm = minor->dev; struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_v3d *v3d = vc4->v3d; - int ret; if (!vc4->v3d) return -ENODEV; - ret = vc4_debugfs_add_file(minor, "v3d_ident", - vc4_v3d_debugfs_ident, NULL); - if (ret) - return ret; + drm_debugfs_add_file(drm, "v3d_ident", vc4_v3d_debugfs_ident, NULL); - ret = vc4_debugfs_add_regset32(minor, "v3d_regs", &v3d->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, "v3d_regs", &v3d->regset); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index f589ab918f4d..d8bf00b442e9 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -716,12 +716,8 @@ static int vc4_vec_late_register(struct drm_encoder *encoder) { struct drm_device *drm = encoder->dev; struct vc4_vec *vec = encoder_to_vc4_vec(encoder); - int ret; - ret = vc4_debugfs_add_regset32(drm->primary, "vec_regs", - &vec->regset); - if (ret) - return ret; + vc4_debugfs_add_regset32(drm, "vec_regs", &vec->regset); return 0; } -- cgit v1.2.3