Hi Sam,
Thanks for the review, I'll address the points left out.
On Sat, Jan 26, 2019 at 04:39:46PM +0100, Sam Ravnborg wrote:
return ret;- }
- /* Reset */
- msleep(20);
- gpiod_set_value(ctx->gpios.power, 1);
- msleep(20);
- gpiod_set_value(ctx->gpios.reset, 1);
- msleep(20);
- return 0;
+}
To verify the above pointer to a datasheet would be nice.
Unfortunately, it's not publicly available :/
+static int rb070d30_panel_unprepare(struct drm_panel *panel) +{
- struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
- gpiod_set_value(ctx->gpios.power, 0);
- gpiod_set_value(ctx->gpios.reset, 0);
- regulator_disable(ctx->supply);
- return 0;
+}
There is sometimes timing constrains after deassert reset.. The order is not strictly reverse of prepare - is that on purpose?
You're right about the order. I couldn't find any delay after a reset though in the datasheet.
+/* Default timings */ +static const struct drm_display_mode default_mode = {
- .clock = 51206,
- .hdisplay = 1024,
- .hsync_start = 1024 + 160,
- .hsync_end = 1024 + 160 + 80,
- .htotal = 1024 + 160 + 80 + 80,
- .vdisplay = 600,
- .vsync_start = 600 + 12,
- .vsync_end = 600 + 12 + 10,
- .vtotal = 600 + 12 + 10 + 13,
- .vrefresh = 60,
+};
height and width missing here. Seems better to add them here than hiding in code below. Is it on purpose that no flags are specified?
+static int rb070d30_panel_get_modes(struct drm_panel *panel) +{
- struct drm_connector *connector = panel->connector;
- struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
- struct drm_display_mode *mode;
- static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
- mode = drm_mode_duplicate(panel->drm, &default_mode);
- if (!mode) {
dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",default_mode.hdisplay, default_mode.vdisplay,default_mode.vrefresh);Use" DRM_DEV_ERROR(&ctx->dsi->dev, "failed to add mode" DRM_MODE_FMT, DRM_MODE_ARG(mode));
return -EINVAL;- }
- drm_mode_set_name(mode);
- mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
- drm_mode_probed_add(connector, mode);
- panel->connector->display_info.bpc = 8;
- panel->connector->display_info.width_mm = 154;
- panel->connector->display_info.height_mm = 85;
See comment on height above. Same goes for bpc
Sorry, I'm not sure to follow you here. bpc and height are both set?
Thanks! Maxime