Re: [PATCH 1/4] drm/i916: add "auto" pipe CRC source

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 01, 2013 at 01:05:34PM +0000, Damien Lespiau wrote:
> On Fri, Nov 01, 2013 at 10:50:20AM +0100, Daniel Vetter wrote:
> > On gmch platforms the normal pipe source CRC registers don't work for
> > DP and TV encoders. And on newer platforms the single pipe CRC has
> > been replaced by a set of CRC at different stages in the platform.
> > 
> > Now most of our userspace tests don't care one bit about the exact
> > CRC, they simply want something that reflects any changes on the
> > screen. Hence add a new auto target for platform agnostic tests to
> > use.
> > 
> > v2: Pass back the adjusted source so that it can be shown in debugfs.
> > 
> > v3: I seem to be unable to get a stable CRC for DP ports. So let's
> > just disable them for now when using the auto mode. Note that
> > testcases need to be restructured so that they can dynamically skip
> > connectors. They also first need to set up the desired mode
> > configuration, since otherwise the auto mode won't do the right thing.
> > 
> > v4: Don't leak the modeset mutex on error paths.
> > 
> > v5: Spelling fix for the i9xx auto_source function.
> > 
> > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> 
> On the series (the improved comments and I sent earlier would be a nice
> addition):
>   Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>

Bikesheds applied and patches merged, thanks for your review.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 91 +++++++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/i915_drv.h     |  1 +
> >  2 files changed, 77 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 5c45e9e..7c29a88 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1936,6 +1936,7 @@ static const char * const pipe_crc_sources[] = {
> >  	"DP-B",
> >  	"DP-C",
> >  	"DP-D",
> > +	"auto",
> >  };
> >  
> >  static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
> > @@ -1964,10 +1965,13 @@ static int display_crc_ctl_open(struct inode *inode, struct file *file)
> >  	return single_open(file, display_crc_ctl_show, dev);
> >  }
> >  
> > -static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> > +static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> >  				 uint32_t *val)
> >  {
> > -	switch (source) {
> > +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> > +		*source = INTEL_PIPE_CRC_SOURCE_PIPE;
> > +
> > +	switch (*source) {
> >  	case INTEL_PIPE_CRC_SOURCE_PIPE:
> >  		*val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX;
> >  		break;
> > @@ -1981,10 +1985,54 @@ static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> >  	return 0;
> >  }
> >  
> > -static int vlv_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> > +static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe,
> > +				     enum intel_pipe_crc_source *source)
> > +{
> > +	struct intel_encoder *encoder;
> > +	struct intel_crtc *crtc;
> > +	int ret = 0;
> > +
> > +	*source = INTEL_PIPE_CRC_SOURCE_PIPE;
> > +
> > +	mutex_lock(&dev->mode_config.mutex);
> > +	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> > +			    base.head) {
> > +		if (!encoder->base.crtc)
> > +			continue;
> > +
> > +		crtc = to_intel_crtc(encoder->base.crtc);
> > +
> > +		if (crtc->pipe != pipe)
> > +			continue;
> > +
> > +		switch (encoder->type) {
> > +		case INTEL_OUTPUT_TVOUT:
> > +			*source = INTEL_PIPE_CRC_SOURCE_TV;
> > +			break;
> > +		case INTEL_OUTPUT_DISPLAYPORT:
> > +		case INTEL_OUTPUT_EDP:
> > +			/* We can't get stable CRCs for DP ports somehow. */
> > +			ret = -ENODEV;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&dev->mode_config.mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static int vlv_pipe_crc_ctl_reg(struct drm_device *dev,
> > +				enum pipe pipe,
> > +				enum intel_pipe_crc_source *source,
> >  				uint32_t *val)
> >  {
> > -	switch (source) {
> > +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) {
> > +		int ret = i9xx_pipe_crc_auto_source(dev, pipe, source);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	switch (*source) {
> >  	case INTEL_PIPE_CRC_SOURCE_PIPE:
> >  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV;
> >  		break;
> > @@ -2005,10 +2053,17 @@ static int vlv_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> >  }
> >  
> >  static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev,
> > -				 enum intel_pipe_crc_source source,
> > +				 enum pipe pipe,
> > +				 enum intel_pipe_crc_source *source,
> >  				 uint32_t *val)
> >  {
> > -	switch (source) {
> > +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) {
> > +		int ret = i9xx_pipe_crc_auto_source(dev, pipe, source);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	switch (*source) {
> >  	case INTEL_PIPE_CRC_SOURCE_PIPE:
> >  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX;
> >  		break;
> > @@ -2042,10 +2097,13 @@ static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev,
> >  	return 0;
> >  }
> >  
> > -static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> > +static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> >  				uint32_t *val)
> >  {
> > -	switch (source) {
> > +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> > +		*source = INTEL_PIPE_CRC_SOURCE_PIPE;
> > +
> > +	switch (*source) {
> >  	case INTEL_PIPE_CRC_SOURCE_PLANE1:
> >  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK;
> >  		break;
> > @@ -2065,10 +2123,13 @@ static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> >  	return 0;
> >  }
> >  
> > -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source source,
> > +static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> >  				uint32_t *val)
> >  {
> > -	switch (source) {
> > +	if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> > +		*source = INTEL_PIPE_CRC_SOURCE_PF;
> > +
> > +	switch (*source) {
> >  	case INTEL_PIPE_CRC_SOURCE_PLANE1:
> >  		*val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB;
> >  		break;
> > @@ -2104,15 +2165,15 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> >  		return -EINVAL;
> >  
> >  	if (IS_GEN2(dev))
> > -		ret = i8xx_pipe_crc_ctl_reg(source, &val);
> > +		ret = i8xx_pipe_crc_ctl_reg(&source, &val);
> >  	else if (INTEL_INFO(dev)->gen < 5)
> > -		ret = i9xx_pipe_crc_ctl_reg(dev, source, &val);
> > +		ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val);
> >  	else if (IS_VALLEYVIEW(dev))
> > -		ret = vlv_pipe_crc_ctl_reg(source, &val);
> > +		ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val);
> >  	else if (IS_GEN5(dev) || IS_GEN6(dev))
> > -		ret = ilk_pipe_crc_ctl_reg(source, &val);
> > +		ret = ilk_pipe_crc_ctl_reg(&source, &val);
> >  	else
> > -		ret = ivb_pipe_crc_ctl_reg(source, &val);
> > +		ret = ivb_pipe_crc_ctl_reg(&source, &val);
> >  
> >  	if (ret != 0)
> >  		return ret;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2c1921d..b12d942 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1253,6 +1253,7 @@ enum intel_pipe_crc_source {
> >  	INTEL_PIPE_CRC_SOURCE_DP_B,
> >  	INTEL_PIPE_CRC_SOURCE_DP_C,
> >  	INTEL_PIPE_CRC_SOURCE_DP_D,
> > +	INTEL_PIPE_CRC_SOURCE_AUTO,
> >  	INTEL_PIPE_CRC_SOURCE_MAX,
> >  };
> >  
> > -- 
> > 1.8.4.rc3
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux