Hi,
this series builds up on the API for exposing captured CRCs through debugfs and also on the refactoring to the analogix_dp driver for using the DP helpers (so it depends on those not-yet merged series).
It adds new DP helpers for starting and stopping CRC capture and gets the Rockchip driver to use it.
Also had to add a connector backpointer to the AUX struct so we could wait for the right vblank and store the CRCs afterwards.
Thanks,
Tomeu
Tomeu Vizoso (5): drm/dp: add connector backpointer to drm_dp_aux drm/bridge: analogix_dp: set connector to drm_dp_aux drm/dp: add helpers for capture of frame CRCs drm/bridge: analogix_dp: add helpers for capture of frame CRCs drm/rockchip: Implement CRC debugfs API
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 34 ++++-- drivers/gpu/drm/drm_dp_helper.c | 133 +++++++++++++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 48 ++++++++ include/drm/bridge/analogix_dp.h | 3 + include/drm/drm_dp_helper.h | 9 ++ 5 files changed, 219 insertions(+), 8 deletions(-)
This backpointer allows DP helpers to access the connector it's being used for.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com ---
include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 63b8bd502444..f66cc8501d71 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -710,6 +710,7 @@ struct drm_dp_aux_msg { * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter * @ddc: I2C adapter that can be used for I2C-over-AUX communication * @dev: pointer to struct device that is the parent for this AUX channel + * @connector: backpointer to connector that uses this AUX channel * @hw_mutex: internal mutex used for locking transfers * @transfer: transfers a message representing a single AUX transaction * @@ -746,6 +747,7 @@ struct drm_dp_aux { const char *name; struct i2c_adapter ddc; struct device *dev; + struct drm_connector *connector; struct mutex hw_mutex; ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg);
On Tue, Aug 09, 2016 at 12:51:32PM +0200, Tomeu Vizoso wrote:
This backpointer allows DP helpers to access the connector it's being used for.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com
include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 63b8bd502444..f66cc8501d71 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -710,6 +710,7 @@ struct drm_dp_aux_msg {
- @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
- @ddc: I2C adapter that can be used for I2C-over-AUX communication
- @dev: pointer to struct device that is the parent for this AUX channel
- @connector: backpointer to connector that uses this AUX channel
- @hw_mutex: internal mutex used for locking transfers
- @transfer: transfers a message representing a single AUX transaction
@@ -746,6 +747,7 @@ struct drm_dp_aux { const char *name; struct i2c_adapter ddc; struct device *dev;
Ah, drm_dp_aux_dev.c uses the dev here, not the connector directly. I think having both is too much, and we instead should replace dev by the newly added connector for consistency in how to get at both the parent device and the connector (which really is the same thing, almost). -Daniel
- struct drm_connector *connector; struct mutex hw_mutex; ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg);
-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Set the backpointer so that the DP helpers are able to access the connector that the drm_dp_aux is associated with.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 624fc4f44450..42fc046e4e9d 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1232,23 +1232,25 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, dp->drm_dev = drm_dev; dp->encoder = dp->plat_data->encoder;
+ ret = analogix_dp_create_bridge(drm_dev, dp); + if (ret) { + DRM_ERROR("failed to create bridge (%d)\n", ret); + goto err_encoder_cleanup; + } + dp->aux.name = "DP-AUX"; dp->aux.transfer = analogix_dpaux_transfer; dp->aux.dev = &pdev->dev; + dp->aux.connector = &dp->connector;
ret = drm_dp_aux_register(&dp->aux); if (ret) - goto err_disable_pm_runtime; - - ret = analogix_dp_create_bridge(drm_dev, dp); - if (ret) { - DRM_ERROR("failed to create bridge (%d)\n", ret); - drm_encoder_cleanup(dp->encoder); - goto err_disable_pm_runtime; - } + goto err_encoder_cleanup;
return 0;
+err_encoder_cleanup: + drm_encoder_cleanup(dp->encoder); err_disable_pm_runtime: pm_runtime_disable(dev);
Adds helpers for starting and stopping capture of frame CRCs through the DPCD. When capture is on, a worker waits for vblanks and retrieves the frame CRC to put it in the queue on the CRTC that is using the eDP connector, so it's passed to userspace.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com ---
drivers/gpu/drm/drm_dp_helper.c | 133 ++++++++++++++++++++++++++++++++++++++++ include/drm/drm_dp_helper.h | 7 +++ 2 files changed, 140 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index eae5ef963cb7..bf88ae7deb9a 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -790,6 +790,90 @@ static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) mutex_unlock(&i2c_to_aux(i2c)->hw_mutex); }
+static int drm_dp_aux_get_crc(struct drm_dp_aux *aux, u8 *crc) +{ + u8 buf, count; + int ret; + + ret = drm_dp_dpcd_readb(aux, DP_TEST_SINK, &buf); + if (ret < 0) + return ret; + + WARN_ON(!(buf & DP_TEST_SINK_START)); + + ret = drm_dp_dpcd_readb(aux, DP_TEST_SINK_MISC, &buf); + if (ret < 0) + return ret; + + count = buf & DP_TEST_COUNT_MASK; + if (count == aux->crc_count) + return -EAGAIN; /* No CRC yet */ + + aux->crc_count = count; + + /* At DP_TEST_CRC_R_CR, there's 6 bytes containing CRC data, 2 bytes + * per component (RGB or CrYCb). + */ + ret = drm_dp_dpcd_read(aux, DP_TEST_CRC_R_CR, crc, 6); + if (ret < 0) + return ret; + + return 0; +} + +static void drm_dp_aux_crc_work(struct work_struct *work) +{ + struct drm_dp_aux *aux = container_of(work, struct drm_dp_aux, + crc_work); + struct drm_crtc *crtc; + wait_queue_head_t *wq; + u8 crc_bytes[6]; + uint32_t crcs[3]; + u32 last; + int ret; + + if (WARN_ON(!aux->connector)) + return; + + crtc = aux->connector->state->crtc; + wq = drm_crtc_vblank_waitqueue(crtc); + while (crtc->crc.opened) { + ret = drm_crtc_vblank_get(crtc); + if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", + crtc->index, ret)) + return; + + last = drm_crtc_vblank_count(crtc); + wait_event(*wq, last != drm_crtc_vblank_count(crtc)); + + drm_crtc_vblank_put(crtc); + + if (!crtc->crc.opened) + break; + + ret = drm_dp_aux_get_crc(aux, crc_bytes); + if (ret == -EAGAIN) { + usleep_range(1000, 2000); + ret = drm_dp_aux_get_crc(aux, crc_bytes); + } + + if (ret) { + DRM_DEBUG_KMS("Failed to get a CRC even after retrying: %d\n", + ret); + continue; + } + + spin_lock_irq(&crtc->crc.lock); + crcs[0] = crc_bytes[0] | crc_bytes[1] << 8; + crcs[1] = crc_bytes[2] | crc_bytes[3] << 8; + crcs[2] = crc_bytes[4] | crc_bytes[5] << 8; + ret = drm_crtc_add_crc_entry(crtc, false, 0, crcs); + spin_unlock_irq(&crtc->crc.lock); + if (!ret) + wake_up_interruptible(&crtc->crc.wq); + } +} + /** * drm_dp_aux_init() - minimally initialise an aux channel * @aux: DisplayPort AUX channel @@ -802,6 +886,7 @@ static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) void drm_dp_aux_init(struct drm_dp_aux *aux) { mutex_init(&aux->hw_mutex); + INIT_WORK(&aux->crc_work, drm_dp_aux_crc_work);
aux->ddc.algo = &drm_dp_i2c_algo; aux->ddc.algo_data = aux; @@ -892,3 +977,51 @@ int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]) EXPORT_SYMBOL(drm_dp_psr_setup_time);
#undef PSR_SETUP_TIME + +/** + * drm_dp_start_crc() - start capture of frame CRCs + * @aux: DisplayPort AUX channel + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_dp_start_crc(struct drm_dp_aux *aux) +{ + u8 buf; + int ret; + + ret = drm_dp_dpcd_readb(aux, DP_TEST_SINK, &buf); + if (ret < 0) + return ret; + + ret = drm_dp_dpcd_writeb(aux, DP_TEST_SINK, buf | DP_TEST_SINK_START); + if (ret < 0) + return ret; + + schedule_work(&aux->crc_work); + + return 0; +} +EXPORT_SYMBOL(drm_dp_start_crc); + +/** + * drm_dp_stop_crc() - stop capture of frame CRCs + * @aux: DisplayPort AUX channel + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_dp_stop_crc(struct drm_dp_aux *aux) +{ + u8 buf; + int ret; + + ret = drm_dp_dpcd_readb(aux, DP_TEST_SINK, &buf); + if (ret < 0) + return ret; + + ret = drm_dp_dpcd_writeb(aux, DP_TEST_SINK, buf & ~DP_TEST_SINK_START); + if (ret < 0) + return ret; + + return 0; +} +EXPORT_SYMBOL(drm_dp_stop_crc); diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index f66cc8501d71..c6ddc4edda61 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -712,6 +712,8 @@ struct drm_dp_aux_msg { * @dev: pointer to struct device that is the parent for this AUX channel * @connector: backpointer to connector that uses this AUX channel * @hw_mutex: internal mutex used for locking transfers + * @crc_work: worker that captures CRCs for each frame + * @crc_count: counter of captured frame CRCs * @transfer: transfers a message representing a single AUX transaction * * The .dev field should be set to a pointer to the device that implements @@ -749,6 +751,8 @@ struct drm_dp_aux { struct device *dev; struct drm_connector *connector; struct mutex hw_mutex; + struct work_struct crc_work; + u8 crc_count; ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg); /** @@ -820,4 +824,7 @@ void drm_dp_aux_init(struct drm_dp_aux *aux); int drm_dp_aux_register(struct drm_dp_aux *aux); void drm_dp_aux_unregister(struct drm_dp_aux *aux);
+int drm_dp_start_crc(struct drm_dp_aux *aux); +int drm_dp_stop_crc(struct drm_dp_aux *aux); + #endif /* _DRM_DP_HELPER_H_ */
Add two simple functions that just take the drm_dp_aux from our struct and calls the corresponding DP helpers with it.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 16 ++++++++++++++++ include/drm/bridge/analogix_dp.h | 3 +++ 2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 42fc046e4e9d..8e6c41a08876 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1313,6 +1313,22 @@ int analogix_dp_resume(struct device *dev) EXPORT_SYMBOL_GPL(analogix_dp_resume); #endif
+int analogix_dp_start_crc(struct drm_connector *connector) +{ + struct analogix_dp_device *dp = to_dp(connector); + + return drm_dp_start_crc(&dp->aux); +} +EXPORT_SYMBOL_GPL(analogix_dp_start_crc); + +int analogix_dp_stop_crc(struct drm_connector *connector) +{ + struct analogix_dp_device *dp = to_dp(connector); + + return drm_dp_stop_crc(&dp->aux); +} +EXPORT_SYMBOL_GPL(analogix_dp_stop_crc); + MODULE_AUTHOR("Jingoo Han jg1.han@samsung.com"); MODULE_DESCRIPTION("Analogix DP Core Driver"); MODULE_LICENSE("GPL v2"); diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 261b86d20e77..e6c535ede838 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -45,4 +45,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, struct analogix_dp_plat_data *plat_data); void analogix_dp_unbind(struct device *dev, struct device *master, void *data);
+int analogix_dp_start_crc(struct drm_connector *connector); +int analogix_dp_stop_crc(struct drm_connector *connector); + #endif /* _ANALOGIX_DP_H_ */
Implement the .set_crc_source() callback and call the DP helpers accordingly to start and stop CRC capture.
This is only done if this CRTC is currently using the eDP connector.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com ---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 91305eb7d312..25f2e7b72982 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -18,6 +18,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_plane_helper.h> +#include <drm/bridge/analogix_dp.h>
#include <linux/kernel.h> #include <linux/module.h> @@ -1084,6 +1085,52 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc, kfree(s); }
+static struct drm_connector *vop_get_edp_connector(struct vop *vop) +{ + struct drm_crtc *crtc = &vop->crtc; + struct drm_connector *connector; + + mutex_lock(&crtc->dev->mode_config.mutex); + drm_for_each_connector(connector, crtc->dev) + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) { + mutex_unlock(&crtc->dev->mode_config.mutex); + return connector; + } + mutex_unlock(&crtc->dev->mode_config.mutex); + + return NULL; +} + +static int vop_crtc_set_crc_source(struct drm_crtc *crtc, + const char *source_name, size_t *values_cnt) +{ + struct vop *vop = to_vop(crtc); + struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc->state); + struct drm_connector *connector; + int ret; + + connector = vop_get_edp_connector(vop); + if (!connector) + return -EINVAL; + + *values_cnt = 3; + + if (source_name && + strcmp(source_name, "auto") == 0) { + + if (s->output_type != DRM_MODE_CONNECTOR_eDP) + return -EINVAL; + + ret = analogix_dp_start_crc(connector); + } else if (!source_name) + + ret = analogix_dp_stop_crc(connector); + else + ret = -EINVAL; + + return ret; +} + static const struct drm_crtc_funcs vop_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, @@ -1091,6 +1138,7 @@ static const struct drm_crtc_funcs vop_crtc_funcs = { .reset = vop_crtc_reset, .atomic_duplicate_state = vop_crtc_duplicate_state, .atomic_destroy_state = vop_crtc_destroy_state, + .set_crc_source = vop_crtc_set_crc_source, };
static bool vop_win_pending_is_complete(struct vop_win *vop_win)
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Tomeu Vizoso