Regards Shashank
On 10/13/2015 6:38 PM, Emil Velikov wrote: > On 10 October 2015 at 06:09, Sharma, Shashank <shashank.sharma at intel.com> > wrote: >> On 10/10/2015 4:37 AM, Emil Velikov wrote: >>> >>> Hi Shashank, >>> >>> On 9 October 2015 at 20:29, Shashank Sharma <shashank.sharma at intel.com> >>> wrote: >>>> >>>> CHV/BSW platform supports two different pipe level gamma >>>> correction modes, which are: >>>> 1. Legacy 8-bit mode >>>> 2. 10-bit CGM (Color Gamut Mapping) mode >>>> >>>> This patch does the following: >>>> 1. Attaches Gamma property to CRTC >>>> 3. Adds the core Gamma correction function for CHV/BSW >>>> 4. Adds Gamma correction macros >>>> >>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com> >>>> Signed-off-by: Kausal Malladi <kausalmalladi at gmail.com> >>>> --- > [snip] >>>> + length = num_samples * sizeof(struct drm_r32g32b32); >>> >>> Calculation can overflow. >> >> good catch, will take care of this. > Actually we do not at all here as the variable is unused. Same applies > for patch 21/22. > Yep, Rob mentioned that in his comment, I have removed this in the latest patch set I have sent. > [snip] >>>> + while (count < num_samples) { >>> >>> Using for(i = 0;....) loop seems the more common approach ? >> >> Nah, we are good with while. The whole color management series prefers while >> (and me too.... :)) > Hmm... so you'd prefer your approach/coding style over the one already > in i915? Feels a bit strange but as long as others are happy fine go > with it. > I am not sure if I915 follows a general rule of using for(...) over while(), coz I see many instances of using a while in i915_gem, i915_drv, i915_irq etc, so it should be good. I would see if someone else can suggest another good reason. > Regards, > Emil >