[Intel-gfx] udpated KMS speed improvement patch

[Jesse Barnes]

Correcting mailing list, should be dri-devel, not drm-devel.  Comments
below.

On Tue, 17 Mar 2009 20:52:21 -0700
Arjan van de Ven <arjan at infradead.org> wrote:

> Latest version below; this is a consolidation of the EDID speed
> improvement and using the async infrastructure...
> ... comments welcome
> 
> With this, the KMS time goes from 1.2 seconds to 0.14 seconds on my
> laptop.
> 
> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c index 1c3a8c5..5950d1a 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -29,6 +29,8 @@
>   *      Jesse Barnes <jesse.barnes at intel.com>
>   */
>  
> +#include <linux/async.h>
> +
>  #include "drmP.h"
>  #include "drm_crtc.h"
>  #include "drm_crtc_helper.h"
> @@ -137,6 +139,26 @@ int drm_helper_probe_connector_modes(struct
> drm_device *dev, uint32_t maxX, }
>  EXPORT_SYMBOL(drm_helper_probe_connector_modes);
>  
> +int drm_helper_probe_connector_modes_fast(struct drm_device *dev,
> uint32_t maxX,
> +				      uint32_t maxY)
> +{
> +	struct drm_connector *connector;
> +	int count = 0;
> +
> +	list_for_each_entry(connector,
> &dev->mode_config.connector_list, head) {
> +		count +=
> drm_helper_probe_single_connector_modes(connector,
> +
> maxX, maxY);
> +		/*
> +		 * If we found a 'good' connector, we stop probing
> futher.
> +		 */
> +		if (count > 0)
> +			break;
> +	}
> +
> +	return count;
> +}
> +EXPORT_SYMBOL(drm_helper_probe_connector_modes_fast);

Ok, this seems fine, though I might call it "probe_one_connector" or
similar, since we return after finding the first connector with mode.

>  /**
>   * drm_initial_config - setup a sane initial connector configuration
>   * @dev: DRM device
> @@ -902,7 +942,7 @@ bool drm_helper_initial_config(struct drm_device
> *dev, bool can_grow) struct drm_connector *connector;
>  	int count = 0;
>  
> -	count = drm_helper_probe_connector_modes(dev,
> +	count = drm_helper_probe_connector_modes_fast(dev,
>  						 dev->mode_config.max_width,
>  						 dev->mode_config.max_height);

This changes behavior a bit though; the idea behind the initial config
is to put the kernel console on as many displays as possible at boot
time, that way people won't miss important messages.  You might argue
that the primary display should be chosen by the low level driver
though; if so we could use that head as the one for kernel messages,
and still get the speedup.

> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a839a28..069b189 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -588,20 +588,22 @@ static unsigned char *drm_ddc_read(struct
> i2c_adapter *adapter) {
>  	struct i2c_algo_bit_data *algo_data = adapter->algo_data;
>  	unsigned char *edid = NULL;
> +	int divider = 5;
>  	int i, j;

Not sure about the DDC changes; we already have problems with not
getting data back on several displays, but I think that problem is
buried in the actual i2c code, not the delays in this routine.

> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index a283427..4b88341 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -319,7 +319,7 @@ void
>  intel_wait_for_vblank(struct drm_device *dev)
>  {
>  	/* Wait for 20ms, i.e. one cycle at 50hz. */
> -	udelay(20000);
> +	mdelay(20);
>  }

We could probably do this independently.  We'll generally be holding the
struct mutex here, but that should be ok.

>  
>  static int
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index 957daef..22a74bd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -81,6 +81,7 @@ struct intel_output {
>  	int type;
>  	struct intel_i2c_chan *i2c_bus; /* for control functions */
>  	struct intel_i2c_chan *ddc_bus; /* for DDC only stuff */
> +	struct edid *edid;
>  	bool load_detect_temp;
>  	bool needs_tv_clock;
>  	void *dev_priv;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c
> b/drivers/gpu/drm/i915/intel_lvds.c index 0d211af..dc4fecc 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -336,6 +336,7 @@ static void intel_lvds_destroy(struct
> drm_connector *connector) intel_i2c_destroy(intel_output->ddc_bus);
>  	drm_sysfs_connector_remove(connector);
>  	drm_connector_cleanup(connector);
> +	kfree(intel_output->edid);
>  	kfree(connector);
>  }
>  
> @@ -516,5 +517,6 @@ failed:
>  	if (intel_output->ddc_bus)
>  		intel_i2c_destroy(intel_output->ddc_bus);
>  	drm_connector_cleanup(connector);
> +	kfree(intel_output->edid);
>  	kfree(connector);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_modes.c
> b/drivers/gpu/drm/i915/intel_modes.c index e42019e..8c0d5f6 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -70,13 +70,21 @@ int intel_ddc_get_modes(struct intel_output
> *intel_output) struct edid *edid;
>  	int ret = 0;
>  
> +	if (intel_output->edid) {
> +		printk(KERN_INFO "Skipping EDID probe due to cached
> edid\n");
> +		return ret;
> +	}
> +
>  	edid = drm_get_edid(&intel_output->base,
>  			    &intel_output->ddc_bus->adapter);
>  	if (edid) {
>  		drm_mode_connector_update_edid_property(&intel_output->base,
>  							edid);
>  		ret = drm_add_edid_modes(&intel_output->base, edid);
> -		kfree(edid);
> +		if (intel_output->type == INTEL_OUTPUT_LVDS)
> +			intel_output->edid = edid;
> +		else
> +			kfree(edid);
>  	}

This also seems ok by itself, though I imagined it in intel_lvds.c
rather than the more generic modes code...

-- 
Jesse Barnes, Intel Open Source Technology Center