Hi Ilia Mirkin,

Thanks for the review.

By old-fashioned way you mean to say using drmModeCrtcSetGamma()? If yes then, 
it shows error as "failed to set gamma: Function no implemented" if any 
platform specific drm has no gamma property implemented.

Current code shows error while running modetest for Xilinx drm as it doesn't 
supports gamma property and ideally it should not show error as gamma is 
optional property, so it doesn't serve the purpose of optional property.

Please correct me if I am missing anything.

Thanks
Rohit

> -----Original Message-----
> From: Ilia Mirkin [mailto:[email protected]]
> Sent: Tuesday, March 3, 2020 7:08 PM
> To: Devarsh Thakkar <[email protected]>
> Cc: Rohit Visavalia <[email protected]>; [email protected];
> [email protected]; Ville Syrjälä <[email protected]>; 
> Hyun
> Kwon <[email protected]>; Ranganathan Sk <[email protected]>; Dhaval
> Rajeshbhai Shah <[email protected]>; Varunkumar Allagadapa
> <[email protected]>
> Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if
> add_property_optional returns true
> 
> EXTERNAL EMAIL
> 
> Pretty sure the current code is right. If the GAMMA_LUT property can't be set,
> it tries to set gamma the old-fashioned way.
> 
> On Tue, Mar 3, 2020 at 8:12 AM Devarsh Thakkar <[email protected]>
> wrote:
> >
> > Hi Rohit,
> >
> > This makes sense to me as gamma was implemented as optional property.
> > Reviewed-By: "Devarsh Thakkar <[email protected]>"
> >
> > @[email protected], @[email protected], @Ville Syrjälä,
> Could you please ack and help merge this patch if it also look good to you ?
> >
> > Regards,
> > Devarsh
> >
> > > -----Original Message-----
> > > From: Rohit Visavalia
> > > Sent: 27 February 2020 00:40
> > > To: Rohit Visavalia <[email protected]>;
> > > [email protected]; [email protected];
> > > [email protected]
> > > Cc: Hyun Kwon <[email protected]>; Ranganathan Sk <[email protected]>;
> > > Dhaval Rajeshbhai Shah <[email protected]>; Varunkumar Allagadapa
> > > <[email protected]>; Devarsh Thakkar <[email protected]>
> > > Subject: RE: [PATCH libdrm] modetest: call drmModeCrtcSetGamma()
> > > only if add_property_optional returns true
> > >
> > > Gentle reminder.
> > >
> > > + Ilia Mirkin, +Emil Velikov.
> > >
> > > Thanks & Regards,
> > > Rohit
> > >
> > > > -----Original Message-----
> > > > From: Rohit Visavalia [mailto:[email protected]]
> > > > Sent: Tuesday, February 25, 2020 3:08 PM
> > > > To: [email protected]
> > > > Cc: Hyun Kwon <[email protected]>; Ranganathan Sk <[email protected]>;
> > > > Dhaval Rajeshbhai Shah <[email protected]>; Varunkumar Allagadapa
> > > > <[email protected]>; Devarsh Thakkar <[email protected]>;
> > > > Rohit Visavalia <[email protected]>
> > > > Subject: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only
> > > > if add_property_optional returns true
> > > >
> > > > gamma is a optional property then also it prints error message, so
> > > > set gamma only if add_property_optional() returns true.
> > > >
> > > > Signed-off-by: Rohit Visavalia <[email protected]>
> > > > ---
> > > >  tests/modetest/modetest.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > > index b907ab3..379b9ea 100644
> > > > --- a/tests/modetest/modetest.c
> > > > +++ b/tests/modetest/modetest.c
> > > > @@ -1138,7 +1138,7 @@ static void set_gamma(struct device *dev,
> > > > unsigned crtc_id, unsigned fourcc)
> > > >
> > > >     add_property_optional(dev, crtc_id, "DEGAMMA_LUT", 0);
> > > >     add_property_optional(dev, crtc_id, "CTM", 0);
> > > > -   if (!add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id)) {
> > > > +   if (add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id))
> > > > + {
> > > >             uint16_t r[256], g[256], b[256];
> > > >
> > > >             for (i = 0; i < 256; i++) {
> > > > --
> > > > 2.7.4
> >
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to