[PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true

Rohit Visavalia RVISAVAL at xilinx.com
Fri Mar 13 10:08:04 UTC 2020


Hi Ilia Mirkin,

But it should not go for setting gamma(drmModeCrtcSetGamma) as user has not asked to do so in just simple mode set command(modetest -M <module> -s 42:3840x2160 at RG16).

What is the requirement for setting gamma drmModeCrtcSetGamma() if user has not asked ?

Thanks
Rohit

From: Ilia Mirkin [mailto:imirkin at alum.mit.edu]
Sent: Thursday, March 12, 2020 3:49 PM
To: Rohit Visavalia <RVISAVAL at xilinx.com>
Cc: Devarsh Thakkar <DEVARSHT at xilinx.com>; dri-devel <dri-devel at lists.freedesktop.org>; emil.velikov at collabora.com; Ville Syrjälä <ville.syrjala at linux.intel.com>; Hyun Kwon <hyunk at xilinx.com>; Ranganathan Sk <rsk at xilinx.com>; Dhaval Rajeshbhai Shah <dshah at xilinx.com>; Varunkumar Allagadapa <VARUNKUM at xilinx.com>
Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true

CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.

Hm. I'm not sure offhand how to check if drmModeCrtcSetGamma is supported. I guess you could check if gamma size > 0 or something?

On Thu, Mar 12, 2020, 02:39 Rohit Visavalia <RVISAVAL at xilinx.com<mailto:RVISAVAL at xilinx.com>> wrote:
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:imirkin at alum.mit.edu<mailto:imirkin at alum.mit.edu>]
> Sent: Tuesday, March 3, 2020 7:08 PM
> To: Devarsh Thakkar <DEVARSHT at xilinx.com<mailto:DEVARSHT at xilinx.com>>
> Cc: Rohit Visavalia <RVISAVAL at xilinx.com<mailto:RVISAVAL at xilinx.com>>; dri-devel at lists.freedesktop.org<mailto:dri-devel at lists.freedesktop.org>;
> emil.velikov at collabora.com<mailto:emil.velikov at collabora.com>; Ville Syrjälä <ville.syrjala at linux.intel.com<mailto:ville.syrjala at linux.intel.com>>; Hyun
> Kwon <hyunk at xilinx.com<mailto:hyunk at xilinx.com>>; Ranganathan Sk <rsk at xilinx.com<mailto:rsk at xilinx.com>>; Dhaval
> Rajeshbhai Shah <dshah at xilinx.com<mailto:dshah at xilinx.com>>; Varunkumar Allagadapa
> <VARUNKUM at xilinx.com<mailto:VARUNKUM at xilinx.com>>
> 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 <DEVARSHT at xilinx.com<mailto:DEVARSHT at xilinx.com>>
> wrote:
> >
> > Hi Rohit,
> >
> > This makes sense to me as gamma was implemented as optional property.
> > Reviewed-By: "Devarsh Thakkar <devarsh.thakkar at xilinx.com<mailto:devarsh.thakkar at xilinx.com>>"
> >
> > @emil.velikov at collabora.com<mailto:emil.velikov at collabora.com>, @imirkin at alum.mit.edu<mailto:imirkin at alum.mit.edu>, @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 <RVISAVAL at xilinx.com<mailto:RVISAVAL at xilinx.com>>;
> > > dri-devel at lists.freedesktop.org<mailto:dri-devel at lists.freedesktop.org>; imirkin at alum.mit.edu<mailto:imirkin at alum.mit.edu>;
> > > emil.velikov at collabora.com<mailto:emil.velikov at collabora.com>
> > > Cc: Hyun Kwon <hyunk at xilinx.com<mailto:hyunk at xilinx.com>>; Ranganathan Sk <rsk at xilinx.com<mailto:rsk at xilinx.com>>;
> > > Dhaval Rajeshbhai Shah <dshah at xilinx.com<mailto:dshah at xilinx.com>>; Varunkumar Allagadapa
> > > <VARUNKUM at xilinx.com<mailto:VARUNKUM at xilinx.com>>; Devarsh Thakkar <DEVARSHT at xilinx.com<mailto:DEVARSHT at xilinx.com>>
> > > 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:rohit.visavalia at xilinx.com<mailto:rohit.visavalia at xilinx.com>]
> > > > Sent: Tuesday, February 25, 2020 3:08 PM
> > > > To: dri-devel at lists.freedesktop.org<mailto:dri-devel at lists.freedesktop.org>
> > > > Cc: Hyun Kwon <hyunk at xilinx.com<mailto:hyunk at xilinx.com>>; Ranganathan Sk <rsk at xilinx.com<mailto:rsk at xilinx.com>>;
> > > > Dhaval Rajeshbhai Shah <dshah at xilinx.com<mailto:dshah at xilinx.com>>; Varunkumar Allagadapa
> > > > <VARUNKUM at xilinx.com<mailto:VARUNKUM at xilinx.com>>; Devarsh Thakkar <DEVARSHT at xilinx.com<mailto:DEVARSHT at xilinx.com>>;
> > > > Rohit Visavalia <RVISAVAL at xilinx.com<mailto:RVISAVAL at xilinx.com>>
> > > > 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 <rohit.visavalia at xilinx.com<mailto:rohit.visavalia at xilinx.com>>
> > > > ---
> > > >  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
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200313/ec1b30a0/attachment-0001.htm>


More information about the dri-devel mailing list