<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix"><tt>On 28-Aug-19 9:25 PM, Shankar, Uma wrote:</tt> </div> <blockquote type="cite" cite="mid:E7C9878FBA1C6D42A1CA3F62AEB6945F821D39B9@BGSMSX104.gar.corp.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">-----Original Message----- From: Sharma, Swati2 Sent: Monday, August 26, 2019 11:56 AM To: <a class="moz-txt-link-abbreviated" href="mailto:intel-gfx@lists.freedesktop.org">intel-gfx@lists.freedesktop.org</a> Cc: Nikula, Jani <a class="moz-txt-link-rfc2396E" href="mailto:jani.nikula@intel.com"><jani.nikula@intel.com></a>; Sharma, Shashank <a class="moz-txt-link-rfc2396E" href="mailto:shashank.sharma@intel.com"><shashank.sharma@intel.com></a>; Manna, Animesh <a class="moz-txt-link-rfc2396E" href="mailto:animesh.manna@intel.com"><animesh.manna@intel.com></a>; Nautiyal, Ankit K <a class="moz-txt-link-rfc2396E" href="mailto:ankit.k.nautiyal@intel.com"><ankit.k.nautiyal@intel.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:daniel.vetter@ffwll.ch">daniel.vetter@ffwll.ch</a>; <a class="moz-txt-link-abbreviated" href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a>; Shankar, Uma <a class="moz-txt-link-rfc2396E" href="mailto:uma.shankar@intel.com"><uma.shankar@intel.com></a>; Sharma, Swati2 <a class="moz-txt-link-rfc2396E" href="mailto:swati2.sharma@intel.com"><swati2.sharma@intel.com></a> Subject: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts() For the legacy gamma, have hw read out to create hw blob of gamma lut values. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Would be better if we define platforms for which this is applicable (I mean what all is considered legacy here) </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Also, add function intel_color_lut_pack to convert hw value with given bit_precision </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Wrap this up within 75 characters. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">to lut property val. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Keep the version history, don't drop that. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Signed-off-by: Swati Sharma <a class="moz-txt-link-rfc2396E" href="mailto:swati2.sharma@intel.com"><swati2.sharma@intel.com></a> --- drivers/gpu/drm/i915/display/intel_color.c | 51 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 ++ 2 files changed, 54 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 27727a1..45e0ee8 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -1521,6 +1521,56 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1, return true; } +/* convert hw value with given bit_precision to lut property val */ +static u32 intel_color_lut_pack(u32 val, u32 bit_precision) { + u32 max = 0xffff >> (16 - bit_precision); + + val = clamp_val(val, 0, max); + + if (bit_precision < 16) + val <<= 16 - bit_precision; + + return val; +} + +static struct drm_property_blob * +i9xx_read_lut_8(struct intel_crtc_state *crtc_state) { </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Would be good to add some comments describing the rationale of this function. Why 8 etc.</pre> </blockquote> <tt>Func is written similar to load luts for i9xx.Therefore didn't explain 8. Do I need to add comment for all the functions/platform? Won't commit message sufficient enough? </tt> <blockquote type="cite" cite="mid:E7C9878FBA1C6D42A1CA3F62AEB6945F821D39B9@BGSMSX104.gar.corp.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum pipe pipe = crtc->pipe; + struct drm_property_blob *blob; + struct drm_color_lut *blob_data; + u32 i, val; + + blob = drm_property_create_blob(&dev_priv->drm, + sizeof(struct drm_color_lut) * 256, </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Have a macro for 256. Explain why 256, add comments.</pre> </blockquote> <tt>This is similar to load luts, since nothing new i added so didn't give explanation. I can re-use LEGACY_LUT_LENGTH for this, since wanted these functions to be similar to load_luts, therefore kept same. </tt> <blockquote type="cite" cite="mid:E7C9878FBA1C6D42A1CA3F62AEB6945F821D39B9@BGSMSX104.gar.corp.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ NULL); + if (IS_ERR(blob)) + return NULL; + + blob_data = blob->data; + + for (i = 0; i < 256; i++) { </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Add the macro for 256.</pre> </blockquote> <tt>Macro already there LEGACY_LUT_LENGTH. Should i reuse that?</tt> <blockquote type="cite" cite="mid:E7C9878FBA1C6D42A1CA3F62AEB6945F821D39B9@BGSMSX104.gar.corp.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ if (HAS_GMCH(dev_priv)) + val = I915_READ(PALETTE(pipe, i)); + else + val = I915_READ(LGC_PALETTE(pipe, i)); + + blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8); + blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8); + blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8); + } + + return blob; +} + +void i9xx_read_luts(struct intel_crtc_state *crtc_state) { + crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state); } + void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1541,6 +1591,7 @@ void intel_color_init(struct intel_crtc *crtc) dev_priv->display.color_check = i9xx_color_check; dev_priv->display.color_commit = i9xx_color_commit; dev_priv->display.load_luts = i9xx_load_luts; + dev_priv->display.read_luts = i9xx_read_luts; } } else { if (INTEL_GEN(dev_priv) >= 11) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a092b34..b687faa 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7192,6 +7192,9 @@ enum { /* legacy palette */ #define _LGC_PALETTE_A 0x4a000 #define _LGC_PALETTE_B 0x4a800 +#define LGC_PALETTE_RED_MASK REG_GENMASK(23, 16) +#define LGC_PALETTE_GREEN_MASK REG_GENMASK(15, 8) +#define LGC_PALETTE_BLUE_MASK REG_GENMASK(7, 0) #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4) /* ilk/snb precision palette */ -- 1.9.1 </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> </pre> </blockquote> <pre class="moz-signature" cols="72">-- ~Swati Sharma </pre> </body> </html>