Re: [PATCH 13/22] drm/i915: CHV: Pipe level Gamma correction

From: "Sharma, Shashank" <shashank.sharma@intel.com>
To: Emil Velikov <emil.l.velikov@gmail.com>
Cc: "Matheson, Annie J" <annie.j.matheson@intel.com>,
	Robert Bradford <robert.bradford@intel.com>,
	"Palleti, Avinash Reddy" <avinash.reddy.palleti@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	ML dri-devel <dri-devel@lists.freedesktop.org>,
	"Mukherjee, Indranil" <indranil.mukherjee@intel.com>,
	Jim Bish <jim.bish@intel.com>,
	"Barnes, Jesse" <jesse.barnes@intel.com>,
	"Smith, Gary K" <gary.k.smith@intel.com>,
	Kausal Malladi <kausalmalladi@gmail.com>,
	"Vetter, Daniel" <daniel.vetter@intel.com>,
	kiran.s.kumar@intel.com
Subject: Re: [PATCH 13/22] drm/i915: CHV: Pipe level Gamma correction
Date: Tue, 13 Oct 2015 19:10:36 +0530	[thread overview]
Message-ID: <561D09D4.5040204@intel.com> (raw)
In-Reply-To: <CACvgo51qdd7GeDa97Bj4W5-+d6E4XZRab4jjuHwo3hMyRj_LOw@mail.gmail.com>

Regards
Shashank

On 10/13/2015 6:38 PM, Emil Velikov wrote:
> On 10 October 2015 at 06:09, Sharma, Shashank <shashank.sharma@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@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@intel.com>
>>>> Signed-off-by: Kausal Malladi <kausalmalladi@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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel