Re: [RFC PATCH 08/12] drm/i915/dsi: add drm mipi dsi host support

[Shobhit Kumar <shobhit.kumar@linux.intel.com>]

On 01/16/2015 05:57 PM, Jani Nikula wrote:
> Add basic support for using the drm mipi dsi framework for DSI. We don't
> use device tree which is pretty much required by mipi_dsi_host_register
> and friends, and we don't have the kind of device model the functions
> expect either. So we cheat and use it as a library to abstract what we
> need: a nice, clean interface for DSI transfers. This means we will have
> to be careful with what functions we call, as the driver model devices
> in mipi_dsi_host and mipi_dsi_device will *not* be initialized.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Looks good.

Reviewed-By: Shobhit Kumar <shobhit.kumar@intel.com>

> ---
>   drivers/gpu/drm/i915/Kconfig               |   1 +
>   drivers/gpu/drm/i915/intel_dsi.c           | 162 ++++++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/intel_dsi.h           |  18 ++++
>   drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   3 -
>   4 files changed, 180 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index da196cd07263..74acca9bcd9d 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -12,6 +12,7 @@ config DRM_I915
>   	select TMPFS
>   	select DRM_KMS_HELPER
>   	select DRM_PANEL
> +	select DRM_MIPI_DSI
>   	# i915 depends on ACPI_VIDEO when ACPI is enabled
>   	# but for select to work, need to select ACPI_VIDEO's dependencies, ick
>   	select BACKLIGHT_LCD_SUPPORT if ACPI
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 19a9955eab0e..5cfa3431785a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -28,6 +28,7 @@
>   #include <drm/drm_edid.h>
>   #include <drm/i915_drm.h>
>   #include <drm/drm_panel.h>
> +#include <drm/drm_mipi_dsi.h>
>   #include <linux/slab.h>
>   #include "i915_drv.h"
>   #include "intel_drv.h"
> @@ -58,6 +59,149 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>   		DRM_ERROR("DPI FIFOs are not empty\n");
>   }
>
> +static void write_data(struct drm_i915_private *dev_priv, u32 reg,
> +		       const u8 *data, u32 len)
> +{
> +	u32 i, j;
> +
> +	for (i = 0; i < len; i += 4) {
> +		u32 val = 0;
> +
> +		for (j = 0; j < min_t(u32, len - i, 4); j++)
> +			val |= *data++ << 8 * j;
> +
> +		I915_WRITE(reg, val);
> +	}
> +}
> +
> +static void read_data(struct drm_i915_private *dev_priv, u32 reg,
> +		      u8 *data, u32 len)
> +{
> +	u32 i, j;
> +
> +	for (i = 0; i < len; i += 4) {
> +		u32 val = I915_READ(reg);
> +
> +		for (j = 0; j < min_t(u32, len - i, 4); j++)
> +			*data++ = val >> 8 * j;
> +	}
> +}
> +
> +static ssize_t intel_dsi_host_transfer(struct mipi_dsi_host *host,
> +				       const struct mipi_dsi_msg *msg)
> +{
> +	struct intel_dsi_host *intel_dsi_host = to_intel_dsi_host(host);
> +	struct drm_device *dev = intel_dsi_host->intel_dsi->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum port port = intel_dsi_host->port;
> +	struct mipi_dsi_packet packet;
> +	ssize_t ret;
> +	const u8 *header, *data;
> +	u32 data_reg, data_mask, ctrl_reg, ctrl_mask;
> +
> +	ret = mipi_dsi_create_packet(&packet, msg);
> +	if (ret < 0)
> +		return ret;
> +
> +	header = packet.header;
> +	data = packet.payload;
> +
> +	if (msg->flags & MIPI_DSI_MSG_USE_LPM) {
> +		data_reg = MIPI_LP_GEN_DATA(port);
> +		data_mask = LP_DATA_FIFO_FULL;
> +		ctrl_reg = MIPI_LP_GEN_CTRL(port);
> +		ctrl_mask = LP_CTRL_FIFO_FULL;
> +	} else {
> +		data_reg = MIPI_HS_GEN_DATA(port);
> +		data_mask = HS_DATA_FIFO_FULL;
> +		ctrl_reg = MIPI_HS_GEN_CTRL(port);
> +		ctrl_mask = HS_CTRL_FIFO_FULL;
> +	}
> +
> +	/* note: this is never true for reads */
> +	if (packet.payload_length) {
> +
> +		if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & data_mask) == 0, 50))
> +			DRM_ERROR("Timeout waiting for HS/LP DATA FIFO !full\n");
> +
> +		write_data(dev_priv, data_reg, packet.payload,
> +			   packet.payload_length);
> +	}
> +
> +	if (msg->rx_len) {
> +		I915_WRITE(MIPI_INTR_STAT(port), GEN_READ_DATA_AVAIL);
> +	}
> +
> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & ctrl_mask) == 0, 50)) {
> +		DRM_ERROR("Timeout waiting for HS/LP CTRL FIFO !full\n");
> +	}
> +
> +	I915_WRITE(ctrl_reg, header[2] << 16 | header[1] << 8 | header[0]);
> +
> +	/* ->rx_len is set only for reads */
> +	if (msg->rx_len) {
> +		data_mask = GEN_READ_DATA_AVAIL;
> +		if (wait_for((I915_READ(MIPI_INTR_STAT(port)) & data_mask) == data_mask, 50))
> +			DRM_ERROR("Timeout waiting for read data.\n");
> +
> +		read_data(dev_priv, data_reg, msg->rx_buf, msg->rx_len);
> +	}
> +
> +	/* XXX: fix for reads and writes */
> +	return 4 + packet.payload_length;
> +}
> +
> +static int intel_dsi_host_attach(struct mipi_dsi_host *host,
> +				 struct mipi_dsi_device *dsi)
> +{
> +	return 0;
> +}
> +
> +static int intel_dsi_host_detach(struct mipi_dsi_host *host,
> +				 struct mipi_dsi_device *dsi)
> +{
> +	return 0;
> +}
> +
> +static const struct mipi_dsi_host_ops intel_dsi_host_ops = {
> +	.attach = intel_dsi_host_attach,
> +	.detach = intel_dsi_host_detach,
> +	.transfer = intel_dsi_host_transfer,
> +};
> +
> +static struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
> +						  enum port port)
> +{
> +	struct intel_dsi_host *host;
> +	struct mipi_dsi_device *device;
> +
> +	host = kzalloc(sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return NULL;
> +
> +	host->base.ops = &intel_dsi_host_ops;
> +	host->intel_dsi = intel_dsi;
> +	host->port = port;
> +
> +	/*
> +	 * We should call mipi_dsi_host_register(&host->base) here, but we don't
> +	 * have a host->dev, and we don't have OF stuff either. So just use the
> +	 * dsi framework as a library and hope for the best. Create the dsi
> +	 * devices by ourselves here too. Need to be careful though, because we
> +	 * don't initialize any of the driver model devices here.
> +	 */
> +	device = kzalloc(sizeof(*device), GFP_KERNEL);
> +	if (!device) {
> +		kfree(host);
> +		return NULL;
> +	}
> +
> +	device->host = &host->base;
> +	host->device = device;
> +
> +	return host;
> +}
> +
>   static void band_gap_reset(struct drm_i915_private *dev_priv)
>   {
>   	mutex_lock(&dev_priv->dpio_lock);
> @@ -806,6 +950,7 @@ void intel_dsi_init(struct drm_device *dev)
>   	struct drm_connector *connector;
>   	struct drm_display_mode *scan, *fixed_mode = NULL;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum port port;
>   	unsigned int i;
>
>   	DRM_DEBUG_KMS("\n");
> @@ -854,7 +999,11 @@ void intel_dsi_init(struct drm_device *dev)
>   	intel_connector->unregister = intel_connector_unregister;
>
>   	/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
> -	if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
> +	if (dev_priv->vbt.dsi.config->dual_link) {
> +		/* XXX: does dual link work on either pipe? */
> +		intel_encoder->crtc_mask = (1 << PIPE_A);
> +		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> +	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
>   		intel_encoder->crtc_mask = (1 << PIPE_A);
>   		intel_dsi->ports = (1 << PORT_A);
>   	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
> @@ -862,6 +1011,17 @@ void intel_dsi_init(struct drm_device *dev)
>   		intel_dsi->ports = (1 << PORT_C);
>   	}
>
> +	/* Create a DSI host (and a device) for each port. */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		struct intel_dsi_host *host;
> +
> +		host = intel_dsi_host_init(intel_dsi, port);
> +		if (!host)
> +			goto err;
> +
> +		intel_dsi->dsi_hosts[port] = host;
> +	}
> +
>   	for (i = 0; i < ARRAY_SIZE(intel_dsi_drivers); i++) {
>   		intel_dsi->panel = intel_dsi_drivers[i].init(intel_dsi,
>   							     intel_dsi_drivers[i].panel_id);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index fc0b2b8d90f1..2784ac442368 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -26,6 +26,7 @@
>
>   #include <drm/drmP.h>
>   #include <drm/drm_crtc.h>
> +#include <drm/drm_mipi_dsi.h>
>   #include "intel_drv.h"
>
>   /* Dual Link support */
> @@ -33,10 +34,13 @@
>   #define DSI_DUAL_LINK_FRONT_BACK	1
>   #define DSI_DUAL_LINK_PIXEL_ALT		2
>
> +struct intel_dsi_host;
> +
>   struct intel_dsi {
>   	struct intel_encoder base;
>
>   	struct drm_panel *panel;
> +	struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
>
>   	struct intel_connector *attached_connector;
>
> @@ -94,6 +98,20 @@ struct intel_dsi {
>   	u16 panel_pwr_cycle_delay;
>   };
>
> +struct intel_dsi_host {
> +	struct mipi_dsi_host base;
> +	struct intel_dsi *intel_dsi;
> +	enum port port;
> +
> +	/* our little hack */
> +	struct mipi_dsi_device *device;
> +};
> +
> +static inline struct intel_dsi_host *to_intel_dsi_host(struct mipi_dsi_host *h)
> +{
> +	return container_of(h, struct intel_dsi_host, base);
> +}
> +
>   #define for_each_dsi_port(__port, __ports_mask) \
>   	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
>   		if ((__ports_mask) & (1 << (__port)))
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 204f54df8fe1..e363c26a2b05 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -399,9 +399,6 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>   	intel_dsi->dual_link = mipi_config->dual_link;
>   	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>
> -	if (intel_dsi->dual_link)
> -		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> -
>   	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>   		bits_per_pixel = 18;
>   	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx