[PATCH 1/2] drm: allow drivers to provide their own EDID fetching routine

Dave Airlie airlied at redhat.com
Tue Jul 20 15:54:30 PDT 2010


On Tue, 2010-07-20 at 15:44 -0700, Jesse Barnes wrote:
> Make drm_edid_read take a new argument, edid_read, to allow drivers to
> provide their own EDID fetch routine.  Export the bit banging DDC over
> i2c version of the EDID fetching routine and make the drivers use it.
> This sets the stage for GMBUS support in the Intel driver.
> 

I think this needs some rework.

You might want to checkout what the radeon driver does for hw i2c
engine. You should set up your own i2c hw handlers and use those instead
of bypassing the i2c stack. GMBUS is just another i2c hw block.

Dave.

> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/drm_edid.c                  |   42 +++++++++++---------------
>  drivers/gpu/drm/i915/intel_drv.h            |    1 +
>  drivers/gpu/drm/i915/intel_hdmi.c           |    2 +-
>  drivers/gpu/drm/i915/intel_modes.c          |    3 +-
>  drivers/gpu/drm/i915/intel_sdvo.c           |    9 ++++--
>  drivers/gpu/drm/nouveau/nouveau_connector.c |    8 ++++-
>  drivers/gpu/drm/radeon/radeon_connectors.c  |    5 ++-
>  drivers/gpu/drm/radeon/radeon_display.c     |    6 ++--
>  include/drm/drm_crtc.h                      |    5 ++-
>  include/drm/drm_edid.h                      |    3 ++
>  10 files changed, 47 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 83d8072..fb720e2 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -214,10 +214,10 @@ EXPORT_SYMBOL(drm_edid_is_valid);
>   *
>   * Try to fetch EDID information by calling i2c driver function.
>   */
> -static int
> -drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
> -		      int block, int len)
> +int
> +drm_ddc_i2c_edid_read(void *data, unsigned char *buf, int block, int len)
>  {
> +	struct i2c_adapter *adapter = data;
>  	unsigned char start = block * EDID_LENGTH;
>  	struct i2c_msg msgs[] = {
>  		{
> @@ -238,9 +238,13 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
>  
>  	return -1;
>  }
> +EXPORT_SYMBOL(drm_ddc_i2c_edid_read);
>  
>  static u8 *
> -drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> +drm_do_get_edid(struct drm_connector *connector,
> +		int (*edid_read)(void *data, unsigned char *buf, int block,
> +				 int len),
> +		void *data)
>  {
>  	int i, j = 0;
>  	u8 *block, *new;
> @@ -250,7 +254,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  
>  	/* base block fetch */
>  	for (i = 0; i < 4; i++) {
> -		if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
> +		if (edid_read(data, block, 0, EDID_LENGTH))
>  			goto out;
>  		if (drm_edid_block_valid(block))
>  			break;
> @@ -269,8 +273,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  
>  	for (j = 1; j <= block[0x7e]; j++) {
>  		for (i = 0; i < 4; i++) {
> -			if (drm_do_probe_ddc_edid(adapter, block, j,
> -						  EDID_LENGTH))
> +			if (edid_read(data, block, j, EDID_LENGTH))
>  				goto out;
>  			if (drm_edid_block_valid(block + j * EDID_LENGTH))
>  				break;
> @@ -291,20 +294,6 @@ out:
>  }
>  
>  /**
> - * Probe DDC presence.
> - *
> - * \param adapter : i2c device adaptor
> - * \return 1 on success
> - */
> -static bool
> -drm_probe_ddc(struct i2c_adapter *adapter)
> -{
> -	unsigned char out;
> -
> -	return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
> -}
> -
> -/**
>   * drm_get_edid - get EDID data, if available
>   * @connector: connector we're probing
>   * @adapter: i2c adapter to use for DDC
> @@ -315,12 +304,17 @@ drm_probe_ddc(struct i2c_adapter *adapter)
>   * Return edid data or NULL if we couldn't find any.
>   */
>  struct edid *drm_get_edid(struct drm_connector *connector,
> -			  struct i2c_adapter *adapter)
> +			  int (*edid_read)(void *data, unsigned char *buf,
> +					   int block, int len),
> +			  void *data)
>  {
>  	struct edid *edid = NULL;
> +	unsigned char out;
>  
> -	if (drm_probe_ddc(adapter))
> -		edid = (struct edid *)drm_do_get_edid(connector, adapter);
> +	/* Check for presence first */
> +	if (edid_read(data, &out, 0, 1) == 0)
> +		edid = (struct edid *)drm_do_get_edid(connector, edid_read,
> +						      data);
>  
>  	connector->display_info.raw_edid = (char *)edid;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3fbedd8..75c7161 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -167,6 +167,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  extern bool intel_ddc_probe(struct intel_encoder *intel_encoder);
>  void intel_i2c_quirk_set(struct drm_device *dev, bool enable);
>  void intel_i2c_reset_gmbus(struct drm_device *dev);
> +int intel_gmbus_get_modes(struct drm_connector *c, int pin);
>  
>  extern void intel_crt_init(struct drm_device *dev);
>  extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 83bd764..f1c8b30 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -143,7 +143,7 @@ intel_hdmi_detect(struct drm_connector *connector)
>  	enum drm_connector_status status = connector_status_disconnected;
>  
>  	hdmi_priv->has_hdmi_sink = false;
> -	edid = drm_get_edid(connector,
> +	edid = drm_get_edid(connector, drm_ddc_i2c_edid_read,
>  			    intel_encoder->ddc_bus);
>  
>  	if (edid) {
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 4b1fd3d..35d15f8 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -26,6 +26,7 @@
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/fb.h>
> +#include <drm/drm_edid.h>
>  #include "drmP.h"
>  #include "intel_drv.h"
>  #include "i915_drv.h"
> @@ -77,7 +78,7 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>  	int ret = 0;
>  
>  	intel_i2c_quirk_set(connector->dev, true);
> -	edid = drm_get_edid(connector, adapter);
> +	edid = drm_get_edid(connector, drm_ddc_i2c_edid_read, adapter);
>  	intel_i2c_quirk_set(connector->dev, false);
>  	if (edid) {
>  		drm_mode_connector_update_edid_property(connector, edid);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 76993ac..0bdb860 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1514,7 +1514,8 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
>  	enum drm_connector_status status = connector_status_connected;
>  	struct edid *edid = NULL;
>  
> -	edid = drm_get_edid(connector, intel_encoder->ddc_bus);
> +	edid = drm_get_edid(connector, drm_ddc_i2c_edid_read,
> +			    intel_encoder->ddc_bus);
>  
>  	/* This is only applied to SDVO cards with multiple outputs */
>  	if (edid == NULL && intel_sdvo_multifunc_encoder(intel_encoder)) {
> @@ -1527,7 +1528,8 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
>  		 */
>  		while(temp_ddc > 1) {
>  			sdvo_priv->ddc_bus = temp_ddc;
> -			edid = drm_get_edid(connector, intel_encoder->ddc_bus);
> +			edid = drm_get_edid(connector, drm_ddc_i2c_edid_read,
> +					    intel_encoder->ddc_bus);
>  			if (edid) {
>  				/*
>  				 * When we can get the EDID, maybe it is the
> @@ -1546,7 +1548,8 @@ intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
>  	 */
>  	if (edid == NULL && sdvo_priv->analog_ddc_bus &&
>  	    !intel_analog_is_connected(connector->dev))
> -		edid = drm_get_edid(connector, sdvo_priv->analog_ddc_bus);
> +		edid = drm_get_edid(connector, drm_ddc_i2c_edid_read,
> +				    sdvo_priv->analog_ddc_bus);
>  
>  	if (edid != NULL) {
>  		bool is_digital = !!(edid->input & DRM_EDID_INPUT_DIGITAL);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 149ed22..69c86e7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -260,7 +260,9 @@ nouveau_connector_detect(struct drm_connector *connector)
>  	i2c = nouveau_connector_ddc_detect(connector, &nv_encoder);
>  	if (i2c) {
>  		nouveau_connector_ddc_prepare(connector, &flags);
> -		nv_connector->edid = drm_get_edid(connector, &i2c->adapter);
> +		nv_connector->edid = drm_get_edid(connector,
> +						  drm_ddc_i2c_edid_read,
> +						  &i2c->adapter);
>  		nouveau_connector_ddc_finish(connector, flags);
>  		drm_mode_connector_update_edid_property(connector,
>  							nv_connector->edid);
> @@ -693,7 +695,9 @@ nouveau_connector_create_lvds(struct drm_device *dev,
>  
>  	if (i2c) {
>  		nouveau_connector_ddc_prepare(connector, &flags);
> -		nv_connector->edid = drm_get_edid(connector, &i2c->adapter);
> +		nv_connector->edid = drm_get_edid(connector,
> +						  drm_ddc_i2c_edid_read,
> +						  &i2c->adapter);
>  		nouveau_connector_ddc_finish(connector, flags);
>  	}
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index f58f8bd..8e129c2 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -489,6 +489,7 @@ static enum drm_connector_status radeon_lvds_detect(struct drm_connector *connec
>  	else {
>  		if (radeon_connector->ddc_bus) {
>  			radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> +							      drm_ddc_i2c_edid_read,
>  							      &radeon_connector->ddc_bus->adapter);
>  			if (radeon_connector->edid)
>  				ret = connector_status_connected;
> @@ -601,7 +602,7 @@ static enum drm_connector_status radeon_vga_detect(struct drm_connector *connect
>  			kfree(radeon_connector->edid);
>  			radeon_connector->edid = NULL;
>  		}
> -		radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
> +		radeon_connector->edid = drm_get_edid(&radeon_connector->base, drm_ddc_i2c_edid_read, &radeon_connector->ddc_bus->adapter);
>  
>  		if (!radeon_connector->edid) {
>  			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
> @@ -753,7 +754,7 @@ static enum drm_connector_status radeon_dvi_detect(struct drm_connector *connect
>  			kfree(radeon_connector->edid);
>  			radeon_connector->edid = NULL;
>  		}
> -		radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
> +		radeon_connector->edid = drm_get_edid(&radeon_connector->base, drm_ddc_i2c_edid_read, &radeon_connector->ddc_bus->adapter);
>  
>  		if (!radeon_connector->edid) {
>  			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index 8154cdf..0f40c93 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -400,12 +400,12 @@ int radeon_ddc_get_modes(struct radeon_connector *radeon_connector)
>  		struct radeon_connector_atom_dig *dig = radeon_connector->con_priv;
>  		if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT ||
>  		     dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && dig->dp_i2c_bus)
> -			radeon_connector->edid = drm_get_edid(&radeon_connector->base, &dig->dp_i2c_bus->adapter);
> +			radeon_connector->edid = drm_get_edid(&radeon_connector->base, drm_ddc_i2c_edid_read, &dig->dp_i2c_bus->adapter);
>  	}
>  	if (!radeon_connector->ddc_bus)
>  		return -1;
>  	if (!radeon_connector->edid) {
> -		radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter);
> +		radeon_connector->edid = drm_get_edid(&radeon_connector->base, drm_ddc_i2c_edid_read, &radeon_connector->ddc_bus->adapter);
>  	}
>  	/* some servers provide a hardcoded edid in rom for KVMs */
>  	if (!radeon_connector->edid)
> @@ -427,7 +427,7 @@ static int radeon_ddc_dump(struct drm_connector *connector)
>  
>  	if (!radeon_connector->ddc_bus)
>  		return -1;
> -	edid = drm_get_edid(connector, &radeon_connector->ddc_bus->adapter);
> +	edid = drm_get_edid(connector, drm_ddc_i2c_edid_read, &radeon_connector->ddc_bus->adapter);
>  	if (edid) {
>  		kfree(edid);
>  	}
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 93a1a31..027286e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -664,7 +664,10 @@ extern char *drm_get_tv_select_name(int val);
>  extern void drm_fb_release(struct drm_file *file_priv);
>  extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group);
>  extern struct edid *drm_get_edid(struct drm_connector *connector,
> -				 struct i2c_adapter *adapter);
> +				 int (*edid_read)(void *data,
> +						  unsigned char *buf,
> +						  int block, int len),
> +				 void *data);
>  extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid);
>  extern void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode);
>  extern void drm_mode_remove(struct drm_connector *connector, struct drm_display_mode *mode);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 39e2cc5..af6ccfa 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -201,4 +201,7 @@ struct edid {
>  
>  #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
>  
> +extern int drm_ddc_i2c_edid_read(void *data, unsigned char *buf, int block,
> +				 int len);
> +
>  #endif /* __DRM_EDID_H__ */




More information about the dri-devel mailing list