| Message ID | 1344446134-3704-4-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 08 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Correctly erase the values previously set and also check for 6pbc and > 10bpc. 6 *bpc*. But is the 6 or 10 bpc usage below correct anyway, as the spec says they are not supported by HDMI or DVI? (Either way, the erase part of the patch is valid.) BR, Jani. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_ddi.c | 26 ++++++++++++++++++++------ > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 896b279..f3fafb8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4308,6 +4308,7 @@ > #define PIPE_DDI_MODE_SELECT_DP_SST (2<<24) > #define PIPE_DDI_MODE_SELECT_DP_MST (3<<24) > #define PIPE_DDI_MODE_SELECT_FDI (4<<24) > +#define PIPE_DDI_BPC_MASK (7<<20) > #define PIPE_DDI_BPC_8 (0<<20) > #define PIPE_DDI_BPC_10 (1<<20) > #define PIPE_DDI_BPC_6 (2<<20) > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 1fbd67c..8b38359 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -725,14 +725,28 @@ void intel_ddi_mode_set(struct drm_encoder *encoder, > /* Enable PIPE_DDI_FUNC_CTL for the pipe to work in HDMI mode */ > temp = I915_READ(DDI_FUNC_CTL(pipe)); > temp &= ~PIPE_DDI_PORT_MASK; > - temp &= ~PIPE_DDI_BPC_12; > + temp &= ~PIPE_DDI_BPC_MASK; > temp &= ~PIPE_DDI_MODE_SELECT_MASK; > temp &= ~(PIPE_DDI_PVSYNC | PIPE_DDI_PHSYNC); > - temp |= PIPE_DDI_SELECT_PORT(port) | > - ((intel_crtc->bpp > 24) ? > - PIPE_DDI_BPC_12 : > - PIPE_DDI_BPC_8) | > - PIPE_DDI_FUNC_ENABLE; > + temp |= PIPE_DDI_FUNC_ENABLE | PIPE_DDI_SELECT_PORT(port); > + > + switch (intel_crtc->bpp) { > + case 18: > + temp |= PIPE_DDI_BPC_6; > + break; > + case 24: > + temp |= PIPE_DDI_BPC_8; > + break; > + case 30: > + temp |= PIPE_DDI_BPC_10; > + break; > + case 36: > + temp |= PIPE_DDI_BPC_12; > + break; > + default: > + WARN(1, "%d bpp unsupported by pipe DDI function\n", > + intel_crtc->bpp); > + } > > if (intel_hdmi->has_hdmi_sink) > temp |= PIPE_DDI_MODE_SELECT_HDMI; > -- > 1.7.11.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Aug 09, 2012 at 12:55:41PM +0300, Jani Nikula wrote: > On Wed, 08 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Correctly erase the values previously set and also check for 6pbc and > > 10bpc. > > 6 *bpc*. But is the 6 or 10 bpc usage below correct anyway, as the spec > says they are not supported by HDMI or DVI? (Either way, the erase part > of the patch is valid.) Iirc the intel_crtc->bpp computation should take these constraints into account. On a quick look intel_choose_pipe_bpp_dither seems to dtrt. -Daniel
Hi 2012/8/9 Daniel Vetter <daniel@ffwll.ch>: > On Thu, Aug 09, 2012 at 12:55:41PM +0300, Jani Nikula wrote: >> On Wed, 08 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote: >> > From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> > >> > Correctly erase the values previously set and also check for 6pbc and >> > 10bpc. >> >> 6 *bpc*. But is the 6 or 10 bpc usage below correct anyway, as the spec >> says they are not supported by HDMI or DVI? (Either way, the erase part >> of the patch is valid.) > > Iirc the intel_crtc->bpp computation should take these constraints into > account. On a quick look intel_choose_pipe_bpp_dither seems to dtrt. Yes, that's exactly what I was checking right now... Also, a quick check on the HDMI spec shows that is does support 30bpp, so 10bpc might be allowed... I really think that inside encoder_mode_set we really shouldn't be doing failure checks anymore since we can't fail and return error codes. Also, my evil plans include using this code for DP too, that's why I covered all cases. Still, I did some checks with testdisplay and it seems our non-24-bpp Kernel code could use some love. > -Daniel > > -- > Daniel Vetter > Mail: daniel@ffwll.ch > Mobile: +41 (0)79 365 57 48
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 896b279..f3fafb8 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4308,6 +4308,7 @@ #define PIPE_DDI_MODE_SELECT_DP_SST (2<<24) #define PIPE_DDI_MODE_SELECT_DP_MST (3<<24) #define PIPE_DDI_MODE_SELECT_FDI (4<<24) +#define PIPE_DDI_BPC_MASK (7<<20) #define PIPE_DDI_BPC_8 (0<<20) #define PIPE_DDI_BPC_10 (1<<20) #define PIPE_DDI_BPC_6 (2<<20) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 1fbd67c..8b38359 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -725,14 +725,28 @@ void intel_ddi_mode_set(struct drm_encoder *encoder, /* Enable PIPE_DDI_FUNC_CTL for the pipe to work in HDMI mode */ temp = I915_READ(DDI_FUNC_CTL(pipe)); temp &= ~PIPE_DDI_PORT_MASK; - temp &= ~PIPE_DDI_BPC_12; + temp &= ~PIPE_DDI_BPC_MASK; temp &= ~PIPE_DDI_MODE_SELECT_MASK; temp &= ~(PIPE_DDI_PVSYNC | PIPE_DDI_PHSYNC); - temp |= PIPE_DDI_SELECT_PORT(port) | - ((intel_crtc->bpp > 24) ? - PIPE_DDI_BPC_12 : - PIPE_DDI_BPC_8) | - PIPE_DDI_FUNC_ENABLE; + temp |= PIPE_DDI_FUNC_ENABLE | PIPE_DDI_SELECT_PORT(port); + + switch (intel_crtc->bpp) { + case 18: + temp |= PIPE_DDI_BPC_6; + break; + case 24: + temp |= PIPE_DDI_BPC_8; + break; + case 30: + temp |= PIPE_DDI_BPC_10; + break; + case 36: + temp |= PIPE_DDI_BPC_12; + break; + default: + WARN(1, "%d bpp unsupported by pipe DDI function\n", + intel_crtc->bpp); + } if (intel_hdmi->has_hdmi_sink) temp |= PIPE_DDI_MODE_SELECT_HDMI;