| Message ID | 1547655697-4092-5-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, Jan 16, 2019 at 09:51:35PM +0530, Uma Shankar wrote: > Enable ICL pipe csc hardware. CSC block is enabled > in CSC_MODE register instead of PLANE_COLOR_CTL. > > ToDO: Extend the ABI to accept 32 bit coefficient values > instead of 16bit for future platforms. > > v2: Addressed Maarten's review comments. > > v3: Addressed Matt's review comments. Removed rmw patterns > as suggested by Matt. > > v4: Addressed Matt's review comments. > > v5: Addressed Ville's review comments. > > v6: Separated pipe output csc programming from regular csc. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 9 ++++++--- > drivers/gpu/drm/i915/intel_color.c | 7 ++++++- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index a84200f..3c3a902 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9861,10 +9861,13 @@ enum skl_power_gate { > #define _PIPE_A_CSC_COEFF_BU 0x4901c > #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 > #define _PIPE_A_CSC_COEFF_BV 0x49024 > + > #define _PIPE_A_CSC_MODE 0x49028 > -#define CSC_BLACK_SCREEN_OFFSET (1 << 2) > -#define CSC_POSITION_BEFORE_GAMMA (1 << 1) > -#define CSC_MODE_YUV_TO_RGB (1 << 0) > +#define ICL_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) > + > #define _PIPE_A_CSC_PREOFF_HI 0x49030 > #define _PIPE_A_CSC_PREOFF_ME 0x49034 > #define _PIPE_A_CSC_PREOFF_LO 0x49038 > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 494891c..9b8d2de 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -134,6 +134,7 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *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); > } Seems like an unintentional change? Otherwise, this looks good now that the output csc for rgb->yuv is moved to the next patch. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > > @@ -242,7 +243,10 @@ static void ilk_load_csc_matrix(struct intel_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), ICL_CSC_ENABLE); > + else > + I915_WRITE(PIPE_CSC_MODE(pipe), 0); > } else { > uint32_t mode = CSC_MODE_YUV_TO_RGB; > > @@ -692,6 +696,7 @@ void intel_color_init(struct intel_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 >
>-----Original Message----- >From: Roper, Matthew D >Sent: Tuesday, January 29, 2019 3:50 AM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten ><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma, >Shashank <shashank.sharma@intel.com> >Subject: Re: [v6 4/6] drm/i915/icl: Enable ICL Pipe CSC block > >On Wed, Jan 16, 2019 at 09:51:35PM +0530, Uma Shankar wrote: >> Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE >> register instead of PLANE_COLOR_CTL. >> >> ToDO: Extend the ABI to accept 32 bit coefficient values instead of >> 16bit for future platforms. >> >> v2: Addressed Maarten's review comments. >> >> v3: Addressed Matt's review comments. Removed rmw patterns as >> suggested by Matt. >> >> v4: Addressed Matt's review comments. >> >> v5: Addressed Ville's review comments. >> >> v6: Separated pipe output csc programming from regular csc. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 9 ++++++--- >> drivers/gpu/drm/i915/intel_color.c | 7 ++++++- >> 2 files changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index a84200f..3c3a902 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -9861,10 +9861,13 @@ enum skl_power_gate { >> #define _PIPE_A_CSC_COEFF_BU 0x4901c >> #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 >> #define _PIPE_A_CSC_COEFF_BV 0x49024 >> + >> #define _PIPE_A_CSC_MODE 0x49028 >> -#define CSC_BLACK_SCREEN_OFFSET (1 << 2) >> -#define CSC_POSITION_BEFORE_GAMMA (1 << 1) >> -#define CSC_MODE_YUV_TO_RGB (1 << 0) >> +#define ICL_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) >> + >> #define _PIPE_A_CSC_PREOFF_HI 0x49030 >> #define _PIPE_A_CSC_PREOFF_ME 0x49034 >> #define _PIPE_A_CSC_PREOFF_LO 0x49038 >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 494891c..9b8d2de 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -134,6 +134,7 @@ static void ilk_load_ycbcr_conversion_matrix(struct >intel_crtc *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); >> } > >Seems like an unintentional change? Ok, will drop this extra line. Did it intentionally though as wanted to separate the co-efficients programming from the mode. It's not related to this patch. Thanks Matt for the review. Regards, Uma Shankar >Otherwise, this looks good now that the output csc for rgb->yuv is moved to the >next patch. > >Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > >> >> @@ -242,7 +243,10 @@ static void ilk_load_csc_matrix(struct intel_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), ICL_CSC_ENABLE); >> + else >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> } else { >> uint32_t mode = CSC_MODE_YUV_TO_RGB; >> >> @@ -692,6 +696,7 @@ void intel_color_init(struct intel_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 >> > >-- >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 a84200f..3c3a902 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9861,10 +9861,13 @@ enum skl_power_gate { #define _PIPE_A_CSC_COEFF_BU 0x4901c #define _PIPE_A_CSC_COEFF_RV_GV 0x49020 #define _PIPE_A_CSC_COEFF_BV 0x49024 + #define _PIPE_A_CSC_MODE 0x49028 -#define CSC_BLACK_SCREEN_OFFSET (1 << 2) -#define CSC_POSITION_BEFORE_GAMMA (1 << 1) -#define CSC_MODE_YUV_TO_RGB (1 << 0) +#define ICL_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) + #define _PIPE_A_CSC_PREOFF_HI 0x49030 #define _PIPE_A_CSC_PREOFF_ME 0x49034 #define _PIPE_A_CSC_PREOFF_LO 0x49038 diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 494891c..9b8d2de 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -134,6 +134,7 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *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); } @@ -242,7 +243,10 @@ static void ilk_load_csc_matrix(struct intel_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), ICL_CSC_ENABLE); + else + I915_WRITE(PIPE_CSC_MODE(pipe), 0); } else { uint32_t mode = CSC_MODE_YUV_TO_RGB; @@ -692,6 +696,7 @@ void intel_color_init(struct intel_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. ToDO: Extend the ABI to accept 32 bit coefficient values instead of 16bit for future platforms. v2: Addressed Maarten's review comments. v3: Addressed Matt's review comments. Removed rmw patterns as suggested by Matt. v4: Addressed Matt's review comments. v5: Addressed Ville's review comments. v6: Separated pipe output csc programming from regular csc. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 9 ++++++--- drivers/gpu/drm/i915/intel_color.c | 7 ++++++- 2 files changed, 12 insertions(+), 4 deletions(-)