On Thu, 16 Feb 2017 11:01:46 +0800, Chris Zhong wrote:
On 02/01/2017 03:22 AM, Sean Paul wrote:
On Sun, Jan 29, 2017 at 01:24:42PM +0000, John Keeping wrote:
Reviewed-by: Sean Paul seanpaul@chromium.org
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com
v3:
- Add Chris' Reviewed-by
Unchanged in v2
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 5bad92e2370e..58cb8ace2fe8 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -82,6 +82,7 @@ #define FRAME_BTA_ACK BIT(14) #define ENABLE_LOW_POWER (0x3f << 8) #define ENABLE_LOW_POWER_MASK (0x3f << 8) +#define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS 0x1 #define VID_MODE_TYPE_BURST_SYNC_PULSES 0x2
This field indicates the video mode transmission type as follows: 00: Non-burst with sync pulses 01: Non-burst with sync events 10 and 11: Burst mode
So, I think define the macro like this is better:
#define VID_MODE_TYPE_NON_BURST_SYNC_PULSES 0x0 #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS 0x1 #define VID_MODE_TYPE_BURST 0x2
#define VID_MODE_TYPE_MASK 0x3
@@ -286,6 +287,7 @@ struct dw_mipi_dsi { u32 format; u16 input_div; u16 feedback_div;
unsigned long mode_flags;
const struct dw_mipi_dsi_plat_data *pdata; };
@@ -551,15 +553,10 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host, return -EINVAL; }
- if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ||
!(device->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
dev_err(dsi->dev, "device mode is unsupported\n");
return -EINVAL;
- }
- dsi->lanes = device->lanes; dsi->channel = device->channel; dsi->format = device->format;
- dsi->mode_flags = device->mode_flags; dsi->panel = of_drm_find_panel(device->dev.of_node); if (dsi->panel) return drm_panel_attach(dsi->panel, &dsi->connector);
@@ -716,7 +713,12 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi) { u32 val;
- val = VID_MODE_TYPE_BURST_SYNC_PULSES | ENABLE_LOW_POWER;
- val = ENABLE_LOW_POWER;
- if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
val |= VID_MODE_TYPE_BURST_SYNC_PULSES;
- else if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) val |= VID_MODE_TYPE_BURST; else if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) val |= VID_MODE_TYPE_NON_BURST_SYNC_PULSES; else val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
OK, this is definitely clearer now that I've forgotten most of the datasheet; without your definitions at the top it's not clear that VID_MODE_TYPE_BURST_SYNC_PULSES is zero.