[RFC][PATCH v2] drm: kirin: Add mode_valid logic to avoid mode clocks we can't generate

From: John Stultz
Date: Tue Jul 18 2017 - 00:24:11 EST


Currently the hikey dsi logic cannot generate accurate byte
clocks values for all pixel clock values. Thus if a mode clock
is selected that cannot match the calculated byte clock, the
device will boot with a blank screen.

This patch uses the new mode_valid callback (many thanks to
Jose Abreu for upstreaming it!) to ensure we don't select
modes we cannot generate.

NOTE: Stylistically I suspect there are better ways to do what
I'm trying to do here. The encoder -> crtc bit is terrible, and
getting the crtc adjusted mode from the encoder logic feels
less then ideal. So feedback would be greatly appreciated!

Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
Cc: David Airlie <airlied@xxxxxxxx>
Cc: Rob Clark <robdclark@xxxxxxxxx>
Cc: Xinliang Liu <xinliang.liu@xxxxxxxxxx>
Cc: Xinliang Liu <z.liuxinliang@xxxxxxxxxxxxx>
Cc: Rongrong Zou <zourongrong@xxxxxxxxx>
Cc: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx>
Cc: Chen Feng <puck.chen@xxxxxxxxxxxxx>
Cc: Jose Abreu <Jose.Abreu@xxxxxxxxxxxx>
Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx>
---
v2: Reworked to calculate if modeclock matches the phy's byteclock,
rather then using a whitelist of known modes.
---
drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 45 +++++++++++++++++++++++++
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 8 +++++
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 2 ++
3 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index f77dcfa..9a553e7 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -24,6 +24,7 @@
#include <drm/drm_encoder_slave.h>
#include <drm/drm_atomic_helper.h>

+#include "kirin_drm_drv.h"
#include "dw_dsi_reg.h"

#define MAX_TX_ESC_CLK 10
@@ -603,6 +604,49 @@ static void dsi_encoder_enable(struct drm_encoder *encoder)
dsi->enable = true;
}

+static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *encoder,
+ const struct drm_display_mode *mode)
+{
+ struct dw_dsi *dsi = encoder_to_dsi(encoder);
+ struct drm_crtc *crtc = NULL;
+ struct mipi_phy_params phy;
+ u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+ u32 adjusted_clock;
+ u32 req_kHz, act_kHz, lane_byte_clk_kHz;
+
+ /* Figure out what the adjusted modeclock will be */
+ /* XXX There's got to be a better way to go encoder->crtc */
+ drm_for_each_crtc(crtc, encoder->dev)
+ if (crtc)
+ break;
+ if (crtc)
+ adjusted_clock = kirin_ade_adj_mode_clk(crtc, mode->clock);
+ else
+ adjusted_clock = mode->clock;
+
+ /* Calculate the lane byte clk using the adjusted mode clk */
+ memset(&phy, 0, sizeof(phy));
+ req_kHz = adjusted_clock * bpp / dsi->lanes;
+ act_kHz = dsi_calc_phy_rate(req_kHz, &phy);
+ lane_byte_clk_kHz = act_kHz / 8;
+
+ DRM_DEBUG_DRIVER("Checking mode %ix%i-%i@%i clock: %i adj_clock: %i...",
+ mode->hdisplay, mode->vdisplay, bpp,
+ drm_mode_vrefresh(mode), mode->clock, adjusted_clock);
+
+ /*
+ * Make sure the adjused mode clock and the lane byte clk
+ * have a common denominator base frequency
+ */
+ if (adjusted_clock/dsi->lanes == lane_byte_clk_kHz/3) {
+ DRM_DEBUG_DRIVER("OK!\n");
+ return MODE_OK;
+ }
+
+ DRM_DEBUG_DRIVER("BAD!\n");
+ return MODE_BAD;
+}
+
static void dsi_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adj_mode)
@@ -622,6 +666,7 @@ static int dsi_encoder_atomic_check(struct drm_encoder *encoder,

static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = {
.atomic_check = dsi_encoder_atomic_check,
+ .mode_valid = dsi_encoder_mode_valid,
.mode_set = dsi_encoder_mode_set,
.enable = dsi_encoder_enable,
.disable = dsi_encoder_disable
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index 074b0af..dffcf76 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -178,6 +178,14 @@ static void ade_init(struct ade_hw_ctx *ctx)
FRM_END_START_MASK, REG_EFFECTIVE_IN_ADEEN_FRMEND);
}

+u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk)
+{
+ struct ade_crtc *acrtc = to_ade_crtc(crtc);
+ struct ade_hw_ctx *ctx = acrtc->ctx;
+
+ return clk_round_rate(ctx->ade_pix_clk, modeclk * 1000) / 1000;
+}
+
static void ade_set_pix_clk(struct ade_hw_ctx *ctx,
struct drm_display_mode *mode,
struct drm_display_mode *adj_mode)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
index 7f60c649..85f69ee 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
@@ -27,4 +27,6 @@ struct kirin_drm_private {

extern const struct kirin_dc_ops ade_dc_ops;

+u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk);
+
#endif /* __KIRIN_DRM_DRV_H__ */
--
2.7.4