Hi Ilia Mirkin, Thanks for the comments.
I have sent new patch for review, can you please check ? Thanks, Rohit > -----Original Message----- > From: Ilia Mirkin [mailto:imir...@alum.mit.edu] > Sent: Friday, March 13, 2020 8:17 PM > To: Rohit Visavalia <rvisa...@xilinx.com> > Cc: Devarsh Thakkar <devar...@xilinx.com>; dri-devel <dri- > de...@lists.freedesktop.org>; emil.veli...@collabora.com; Ville Syrjälä > <ville.syrj...@linux.intel.com>; Hyun Kwon <hy...@xilinx.com>; Ranganathan > Sk <r...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>; > Varunkumar Allagadapa <varun...@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. > > > It's to restore the gamma ramp to be the default linear one. Let's say that > the > driver doesn't have the GAMMA_LUT property support, and then you want to > modeset with C8 (indexed) format. That means that modeset has to set the > LUT to make the indexed lookups work (which it does, it all works, you > celebrate). Then you want to run modeset with e.g. > XR24, and the colors are all black! The LUT is persistent across modesets, so > it > has to reset the ramp to linear. > > You could condition calling set_gamma on crtc->gamma_size > 0, I think > -- it didn't occur to me that there would be drivers that didn't support a > LUT. > > On Fri, Mar 13, 2020 at 6:08 AM Rohit Visavalia <rvisa...@xilinx.com> wrote: > > > > 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@RG16). > > > > > > > > What is the requirement for setting gamma drmModeCrtcSetGamma() if user > has not asked ? > > > > > > > > Thanks > > Rohit > > > > > > > > From: Ilia Mirkin [mailto:imir...@alum.mit.edu] > > Sent: Thursday, March 12, 2020 3:49 PM > > To: Rohit Visavalia <rvisa...@xilinx.com> > > Cc: Devarsh Thakkar <devar...@xilinx.com>; dri-devel > > <dri-devel@lists.freedesktop.org>; emil.veli...@collabora.com; Ville > > Syrjälä <ville.syrj...@linux.intel.com>; Hyun Kwon <hy...@xilinx.com>; > > Ranganathan Sk <r...@xilinx.com>; Dhaval Rajeshbhai Shah > > <ds...@xilinx.com>; Varunkumar Allagadapa <varun...@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 <rvisa...@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:imir...@alum.mit.edu] > > > Sent: Tuesday, March 3, 2020 7:08 PM > > > To: Devarsh Thakkar <devar...@xilinx.com> > > > Cc: Rohit Visavalia <rvisa...@xilinx.com>; > > > dri-devel@lists.freedesktop.org; emil.veli...@collabora.com; Ville > > > Syrjälä <ville.syrj...@linux.intel.com>; Hyun Kwon > > > <hy...@xilinx.com>; Ranganathan Sk <r...@xilinx.com>; Dhaval > > > Rajeshbhai Shah <ds...@xilinx.com>; Varunkumar Allagadapa > > > <varun...@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 <devar...@xilinx.com> > > > wrote: > > > > > > > > Hi Rohit, > > > > > > > > This makes sense to me as gamma was implemented as optional > property. > > > > Reviewed-By: "Devarsh Thakkar <devarsh.thak...@xilinx.com>" > > > > > > > > @emil.veli...@collabora.com, @imir...@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 <rvisa...@xilinx.com>; > > > > > dri-devel@lists.freedesktop.org; imir...@alum.mit.edu; > > > > > emil.veli...@collabora.com > > > > > Cc: Hyun Kwon <hy...@xilinx.com>; Ranganathan Sk > > > > > <r...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>; > > > > > Varunkumar Allagadapa <varun...@xilinx.com>; Devarsh Thakkar > > > > > <devar...@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.visava...@xilinx.com] > > > > > > Sent: Tuesday, February 25, 2020 3:08 PM > > > > > > To: dri-devel@lists.freedesktop.org > > > > > > Cc: Hyun Kwon <hy...@xilinx.com>; Ranganathan Sk > > > > > > <r...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>; > > > > > > Varunkumar Allagadapa <varun...@xilinx.com>; Devarsh Thakkar > > > > > > <devar...@xilinx.com>; Rohit Visavalia <rvisa...@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.visava...@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 > > > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel