| Message ID | 1540393213-11907-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Gen 11 pipe color features | expand |
On Wed, Oct 24, 2018 at 08:30:12PM +0530, Uma Shankar wrote: > Enable ICL pipe csc hardware. CSC block is enabled > in CSC_MODE register instead of PLANE_COLOR_CTL. > > v2: Addressed Maarten's review comments. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_color.c | 14 ++++++++++++-- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3adf689..857ce79 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9700,6 +9700,7 @@ enum skl_power_gate { > #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 > #define _PIPE_A_CSC_COEFF_BV 0x49024 > #define _PIPE_A_CSC_MODE 0x49028 > +#define CSC_ENABLE (1 << 31) > #define CSC_BLACK_SCREEN_OFFSET (1 << 2) > #define CSC_POSITION_BEFORE_GAMMA (1 << 1) > #define CSC_MODE_YUV_TO_RGB (1 << 0) > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 12c659f..e7042fd 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -129,7 +129,12 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) > I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); > I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); > I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); > - I915_WRITE(PIPE_CSC_MODE(pipe), 0); > + > + if (INTEL_GEN(dev_priv) >= 11) > + I915_WRITE(PIPE_CSC_MODE(pipe), > + I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE); Is this the right approach for gen11? The bspec details are pretty sparse, but it looks like with gen11+, we now have two CSC's so we can do both a userspace-provided CSC matrix (bit 31 in CSC_MODE) and a separate RGB->YUV matrix (bit 30 in CSC_MODE and the OUTPUT_CSC_* registers) rather than having to pick just one or the other. Also, as noted on the other patch, we generally don't want to do read-modify-write patterns in the driver. Matt > + else > + I915_WRITE(PIPE_CSC_MODE(pipe), 0); > } > > static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) > @@ -239,7 +244,11 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) > I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); > I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); > > - I915_WRITE(PIPE_CSC_MODE(pipe), 0); > + if (INTEL_GEN(dev_priv) >= 11) > + I915_WRITE(PIPE_CSC_MODE(pipe), > + I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE); > + else > + I915_WRITE(PIPE_CSC_MODE(pipe), 0); > } else { > uint32_t mode = CSC_MODE_YUV_TO_RGB; > > @@ -735,6 +744,7 @@ void intel_color_init(struct drm_crtc *crtc) > dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; > dev_priv->display.load_luts = glk_load_luts; > } else if (IS_ICELAKE(dev_priv)) { > + dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; > dev_priv->display.load_luts = icl_load_luts; > } else { > dev_priv->display.load_luts = i9xx_load_luts; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Roper, Matthew D >Sent: Thursday, November 1, 2018 5:10 AM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>; >Lankhorst, Maarten <maarten.lankhorst@intel.com> >Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915/icl: Enable ICL Pipe CSC block > >On Wed, Oct 24, 2018 at 08:30:12PM +0530, Uma Shankar wrote: >> Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE >> register instead of PLANE_COLOR_CTL. >> >> v2: Addressed Maarten's review comments. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/intel_color.c | 14 ++++++++++++-- >> 2 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 3adf689..857ce79 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -9700,6 +9700,7 @@ enum skl_power_gate { >> #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 >> #define _PIPE_A_CSC_COEFF_BV 0x49024 >> #define _PIPE_A_CSC_MODE 0x49028 >> +#define CSC_ENABLE (1 << 31) >> #define CSC_BLACK_SCREEN_OFFSET (1 << 2) >> #define CSC_POSITION_BEFORE_GAMMA (1 << 1) >> #define CSC_MODE_YUV_TO_RGB (1 << 0) >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 12c659f..e7042fd 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -129,7 +129,12 @@ static void ilk_load_ycbcr_conversion_matrix(struct >intel_crtc *intel_crtc) >> I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); >> I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), >POSTOFF_RGB_TO_YUV_ME); >> I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); >> - I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> + >> + if (INTEL_GEN(dev_priv) >= 11) >> + I915_WRITE(PIPE_CSC_MODE(pipe), >> + I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE); > >Is this the right approach for gen11? The bspec details are pretty sparse, but it >looks like with gen11+, we now have two CSC's so we can do both a userspace- >provided CSC matrix (bit 31 in CSC_MODE) and a separate RGB->YUV matrix (bit >30 in CSC_MODE and the OUTPUT_CSC_* >registers) rather than having to pick just one or the other. Yes, actually for RGB->YUV output CSC should be used. So this should set but 30 instead of normal CSC. As it allows final conversion on non-linear data coming out of pipe after blending and gamma operation. Will correct this. Thanks !!! >Also, as noted on the other patch, we generally don't want to do read-modify- >write patterns in the driver. Yes, will change this. Regards, Uma Shankar > > >Matt > >> + else >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> } >> >> static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) @@ >> -239,7 +244,11 @@ static void ilk_load_csc_matrix(struct drm_crtc_state >*crtc_state) >> I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); >> I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); >> >> - I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> + if (INTEL_GEN(dev_priv) >= 11) >> + I915_WRITE(PIPE_CSC_MODE(pipe), >> + I915_READ(PIPE_CSC_MODE(pipe)) | >CSC_ENABLE); >> + else >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> } else { >> uint32_t mode = CSC_MODE_YUV_TO_RGB; >> >> @@ -735,6 +744,7 @@ void intel_color_init(struct drm_crtc *crtc) >> dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; >> dev_priv->display.load_luts = glk_load_luts; >> } else if (IS_ICELAKE(dev_priv)) { >> + dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; >> dev_priv->display.load_luts = icl_load_luts; >> } else { >> dev_priv->display.load_luts = i9xx_load_luts; >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Matt Roper >Graphics Software Engineer >IoTG Platform Enabling & Development >Intel Corporation >(916) 356-2795
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3adf689..857ce79 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9700,6 +9700,7 @@ enum skl_power_gate { #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 #define _PIPE_A_CSC_COEFF_BV 0x49024 #define _PIPE_A_CSC_MODE 0x49028 +#define CSC_ENABLE (1 << 31) #define CSC_BLACK_SCREEN_OFFSET (1 << 2) #define CSC_POSITION_BEFORE_GAMMA (1 << 1) #define CSC_MODE_YUV_TO_RGB (1 << 0) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 12c659f..e7042fd 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -129,7 +129,12 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); - I915_WRITE(PIPE_CSC_MODE(pipe), 0); + + if (INTEL_GEN(dev_priv) >= 11) + I915_WRITE(PIPE_CSC_MODE(pipe), + I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE); + else + I915_WRITE(PIPE_CSC_MODE(pipe), 0); } static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) @@ -239,7 +244,11 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); - I915_WRITE(PIPE_CSC_MODE(pipe), 0); + if (INTEL_GEN(dev_priv) >= 11) + I915_WRITE(PIPE_CSC_MODE(pipe), + I915_READ(PIPE_CSC_MODE(pipe)) | CSC_ENABLE); + else + I915_WRITE(PIPE_CSC_MODE(pipe), 0); } else { uint32_t mode = CSC_MODE_YUV_TO_RGB; @@ -735,6 +744,7 @@ void intel_color_init(struct drm_crtc *crtc) dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; dev_priv->display.load_luts = glk_load_luts; } else if (IS_ICELAKE(dev_priv)) { + dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; dev_priv->display.load_luts = icl_load_luts; } else { dev_priv->display.load_luts = i9xx_load_luts;
Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE register instead of PLANE_COLOR_CTL. v2: Addressed Maarten's review comments. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_color.c | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)