dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Ilia Mirkin <imirkin@alum.mit.edu>
To: Rohit Visavalia <RVISAVAL@xilinx.com>
Cc: Ranganathan Sk <rsk@xilinx.com>,
	Dhaval Rajeshbhai Shah <dshah@xilinx.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Varunkumar Allagadapa <VARUNKUM@xilinx.com>,
	Devarsh Thakkar <DEVARSHT@xilinx.com>,
	"emil.velikov@collabora.com" <emil.velikov@collabora.com>
Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true
Date: Thu, 12 Mar 2020 06:18:53 -0400	[thread overview]
Message-ID: <CAKb7UviVGAZKAhsyEy8JLK6MSrMWDOypELM=OED1hib-MEgusw@mail.gmail.com> (raw)
In-Reply-To: <BYAPR02MB4056A1136D4354043D95B2E9B3FD0@BYAPR02MB4056.namprd02.prod.outlook.com>


[-- Attachment #1.1: Type: text/plain, Size: 4591 bytes --]

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@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@alum.mit.edu]
> > Sent: Tuesday, March 3, 2020 7:08 PM
> > To: Devarsh Thakkar <DEVARSHT@xilinx.com>
> > Cc: Rohit Visavalia <RVISAVAL@xilinx.com>;
> dri-devel@lists.freedesktop.org;
> > emil.velikov@collabora.com; Ville Syrjälä <ville.syrjala@linux.intel.com>;
> Hyun
> > Kwon <hyunk@xilinx.com>; Ranganathan Sk <rsk@xilinx.com>; Dhaval
> > Rajeshbhai Shah <dshah@xilinx.com>; Varunkumar Allagadapa
> > <VARUNKUM@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@xilinx.com>
> > wrote:
> > >
> > > Hi Rohit,
> > >
> > > This makes sense to me as gamma was implemented as optional property.
> > > Reviewed-By: "Devarsh Thakkar <devarsh.thakkar@xilinx.com>"
> > >
> > > @emil.velikov@collabora.com, @imirkin@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@xilinx.com>;
> > > > dri-devel@lists.freedesktop.org; imirkin@alum.mit.edu;
> > > > emil.velikov@collabora.com
> > > > Cc: Hyun Kwon <hyunk@xilinx.com>; Ranganathan Sk <rsk@xilinx.com>;
> > > > Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Varunkumar Allagadapa
> > > > <VARUNKUM@xilinx.com>; Devarsh Thakkar <DEVARSHT@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@xilinx.com]
> > > > > Sent: Tuesday, February 25, 2020 3:08 PM
> > > > > To: dri-devel@lists.freedesktop.org
> > > > > Cc: Hyun Kwon <hyunk@xilinx.com>; Ranganathan Sk <rsk@xilinx.com>;
> > > > > Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Varunkumar Allagadapa
> > > > > <VARUNKUM@xilinx.com>; Devarsh Thakkar <DEVARSHT@xilinx.com>;
> > > > > Rohit Visavalia <RVISAVAL@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@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
> > >
>

[-- Attachment #1.2: Type: text/html, Size: 8429 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-03-12 10:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-25  9:38 [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true Rohit Visavalia
2020-02-27  8:40 ` Rohit Visavalia
2020-03-03 13:11   ` Devarsh Thakkar
2020-03-03 13:38     ` Ilia Mirkin
2020-03-12  6:39       ` Rohit Visavalia
2020-03-12 10:18         ` Ilia Mirkin [this message]
2020-03-13 10:08           ` Rohit Visavalia
2020-03-13 14:46             ` Ilia Mirkin
2020-03-23  4:25               ` Rohit Visavalia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKb7UviVGAZKAhsyEy8JLK6MSrMWDOypELM=OED1hib-MEgusw@mail.gmail.com' \
    --to=imirkin@alum.mit.edu \
    --cc=DEVARSHT@xilinx.com \
    --cc=RVISAVAL@xilinx.com \
    --cc=VARUNKUM@xilinx.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dshah@xilinx.com \
    --cc=emil.velikov@collabora.com \
    --cc=rsk@xilinx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).