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
