From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic_check
Date: Fri, 3 Mar 2017 13:27:30 +0000 [thread overview]
Message-ID: <1488547649.2940.5.camel@synopsys.com> (raw)
In-Reply-To: <20170302195437.mlhzia2q2oav27mr@phenom.ffwll.local>
Hi Daniel,
On Thu, 2017-03-02@20:54 +0100, Daniel Vetter wrote:
> On Thu, Mar 02, 2017@08:27:54PM +0300, Alexey Brodkin wrote:
> >
> > Since we cannot always generate exactly requested pixel clock
> > there's not much sense in checking requested_clock == clk_round_rate().
> > In that case for quite some modes we'll be getting -EINVAL and no video
> > output at all.
> >
> > But given there's some tolerance to real pixel clock in TVs/monitors
> > we may still give it a try with the clock as close to requested one as
> > PLL on the board may generate. So we just do a fixup to what current
> > board may provide.
> >
> > Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: David Airlie <airlied at linux.ie>
> > Cc: Jose Abreu <joabreu at synopsys.com>
> > ---
> > ?drivers/gpu/drm/arc/arcpgu_crtc.c | 16 +++++++---------
> > ?1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index ad9a95916f1f..3f2823c1efc3 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -129,18 +129,16 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
> > ? ??????~ARCPGU_CTRL_ENABLE_MASK);
> > ?}
> > ?
> > -static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
> > - ?????struct drm_crtc_state *state)
> > +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc,
> > + ????const struct drm_display_mode *mode,
> > + ????struct drm_display_mode *adjusted_mode)
>
> This isn't required at all, see drm_crtc_state.adjusted_mode. Just update
> that and you're good - .mode_fixup is the backwards compatibility function
> for old kms drivers, atomic_check is strictly more powerful.
So if I understood you correct here what I really need is just to get rid of existing check,
right? I.e. the following is to be in v2 respin:
------------------------------->8-------------------------------
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a95916f1f..86f1555914e8 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -129,20 +129,6 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
??????????????????????????????~ARCPGU_CTRL_ENABLE_MASK);
?}
?
-static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
-????????????????????????????????????struct drm_crtc_state *state)
-{
-???????struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
-???????struct drm_display_mode *mode = &state->adjusted_mode;
-???????long rate, clk_rate = mode->clock * 1000;
-
-???????rate = clk_round_rate(arcpgu->clk, clk_rate);
-???????if (rate != clk_rate)
-???????????????return -EINVAL;
-
-???????return 0;
-}
-
?static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
??????????????????????????????????????struct drm_crtc_state *state)
?{
@@ -165,7 +151,6 @@ static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
????????.disable????????= arc_pgu_crtc_disable,
????????.prepare????????= arc_pgu_crtc_disable,
????????.commit?????????= arc_pgu_crtc_enable,
-???????.atomic_check???= arc_pgu_crtc_atomic_check,
????????.atomic_begin???= arc_pgu_crtc_atomic_begin,
?};
------------------------------->8-------------------------------
> Please also make sure the documentation properly explains this, and if
> not, please submit a patch to improve it.
You mean explains what? That .mode_fixup is not meant to be used in
new code?
-Alexey
WARNING: multiple messages have this Message-ID (diff)
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: "Jose.Abreu@synopsys.com" <Jose.Abreu@synopsys.com>,
"airlied@linux.ie" <airlied@linux.ie>,
"daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-snps-arc@lists.infradead.org"
<linux-snps-arc@lists.infradead.org>
Subject: Re: [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic_check
Date: Fri, 3 Mar 2017 13:27:30 +0000 [thread overview]
Message-ID: <1488547649.2940.5.camel@synopsys.com> (raw)
In-Reply-To: <20170302195437.mlhzia2q2oav27mr@phenom.ffwll.local>
Hi Daniel,
On Thu, 2017-03-02 at 20:54 +0100, Daniel Vetter wrote:
> On Thu, Mar 02, 2017 at 08:27:54PM +0300, Alexey Brodkin wrote:
> >
> > Since we cannot always generate exactly requested pixel clock
> > there's not much sense in checking requested_clock == clk_round_rate().
> > In that case for quite some modes we'll be getting -EINVAL and no video
> > output at all.
> >
> > But given there's some tolerance to real pixel clock in TVs/monitors
> > we may still give it a try with the clock as close to requested one as
> > PLL on the board may generate. So we just do a fixup to what current
> > board may provide.
> >
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Jose Abreu <joabreu@synopsys.com>
> > ---
> > drivers/gpu/drm/arc/arcpgu_crtc.c | 16 +++++++---------
> > 1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index ad9a95916f1f..3f2823c1efc3 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -129,18 +129,16 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
> > ~ARCPGU_CTRL_ENABLE_MASK);
> > }
> >
> > -static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
> > - struct drm_crtc_state *state)
> > +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
>
> This isn't required at all, see drm_crtc_state.adjusted_mode. Just update
> that and you're good - .mode_fixup is the backwards compatibility function
> for old kms drivers, atomic_check is strictly more powerful.
So if I understood you correct here what I really need is just to get rid of existing check,
right? I.e. the following is to be in v2 respin:
------------------------------->8-------------------------------
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a95916f1f..86f1555914e8 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -129,20 +129,6 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
~ARCPGU_CTRL_ENABLE_MASK);
}
-static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
- struct drm_crtc_state *state)
-{
- struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
- struct drm_display_mode *mode = &state->adjusted_mode;
- long rate, clk_rate = mode->clock * 1000;
-
- rate = clk_round_rate(arcpgu->clk, clk_rate);
- if (rate != clk_rate)
- return -EINVAL;
-
- return 0;
-}
-
static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
@@ -165,7 +151,6 @@ static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
.disable = arc_pgu_crtc_disable,
.prepare = arc_pgu_crtc_disable,
.commit = arc_pgu_crtc_enable,
- .atomic_check = arc_pgu_crtc_atomic_check,
.atomic_begin = arc_pgu_crtc_atomic_begin,
};
------------------------------->8-------------------------------
> Please also make sure the documentation properly explains this, and if
> not, please submit a patch to improve it.
You mean explains what? That .mode_fixup is not meant to be used in
new code?
-Alexey
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
WARNING: multiple messages have this Message-ID (diff)
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: "dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Jose.Abreu@synopsys.com" <Jose.Abreu@synopsys.com>,
"daniel.vetter@ffwll.ch" <daniel.vetter@ffwll.ch>,
"linux-snps-arc@lists.infradead.org"
<linux-snps-arc@lists.infradead.org>,
"airlied@linux.ie" <airlied@linux.ie>
Subject: Re: [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic_check
Date: Fri, 3 Mar 2017 13:27:30 +0000 [thread overview]
Message-ID: <1488547649.2940.5.camel@synopsys.com> (raw)
In-Reply-To: <20170302195437.mlhzia2q2oav27mr@phenom.ffwll.local>
Hi Daniel,
On Thu, 2017-03-02 at 20:54 +0100, Daniel Vetter wrote:
> On Thu, Mar 02, 2017 at 08:27:54PM +0300, Alexey Brodkin wrote:
> >
> > Since we cannot always generate exactly requested pixel clock
> > there's not much sense in checking requested_clock == clk_round_rate().
> > In that case for quite some modes we'll be getting -EINVAL and no video
> > output at all.
> >
> > But given there's some tolerance to real pixel clock in TVs/monitors
> > we may still give it a try with the clock as close to requested one as
> > PLL on the board may generate. So we just do a fixup to what current
> > board may provide.
> >
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Jose Abreu <joabreu@synopsys.com>
> > ---
> > drivers/gpu/drm/arc/arcpgu_crtc.c | 16 +++++++---------
> > 1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index ad9a95916f1f..3f2823c1efc3 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -129,18 +129,16 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
> > ~ARCPGU_CTRL_ENABLE_MASK);
> > }
> >
> > -static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
> > - struct drm_crtc_state *state)
> > +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
>
> This isn't required at all, see drm_crtc_state.adjusted_mode. Just update
> that and you're good - .mode_fixup is the backwards compatibility function
> for old kms drivers, atomic_check is strictly more powerful.
So if I understood you correct here what I really need is just to get rid of existing check,
right? I.e. the following is to be in v2 respin:
------------------------------->8-------------------------------
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a95916f1f..86f1555914e8 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -129,20 +129,6 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
~ARCPGU_CTRL_ENABLE_MASK);
}
-static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
- struct drm_crtc_state *state)
-{
- struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
- struct drm_display_mode *mode = &state->adjusted_mode;
- long rate, clk_rate = mode->clock * 1000;
-
- rate = clk_round_rate(arcpgu->clk, clk_rate);
- if (rate != clk_rate)
- return -EINVAL;
-
- return 0;
-}
-
static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
@@ -165,7 +151,6 @@ static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
.disable = arc_pgu_crtc_disable,
.prepare = arc_pgu_crtc_disable,
.commit = arc_pgu_crtc_enable,
- .atomic_check = arc_pgu_crtc_atomic_check,
.atomic_begin = arc_pgu_crtc_atomic_begin,
};
------------------------------->8-------------------------------
> Please also make sure the documentation properly explains this, and if
> not, please submit a patch to improve it.
You mean explains what? That .mode_fixup is not meant to be used in
new code?
-Alexey
next prev parent reply other threads:[~2017-03-03 13:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-02 17:27 [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic_check Alexey Brodkin
2017-03-02 17:27 ` Alexey Brodkin
2017-03-02 17:27 ` Alexey Brodkin
2017-03-02 19:54 ` Daniel Vetter
2017-03-02 19:54 ` Daniel Vetter
2017-03-02 19:54 ` Daniel Vetter
2017-03-03 13:27 ` Alexey Brodkin [this message]
2017-03-03 13:27 ` Alexey Brodkin
2017-03-03 13:27 ` Alexey Brodkin
2017-03-03 18:05 ` Jose Abreu
2017-03-03 18:05 ` Jose Abreu
2017-03-03 18:05 ` Jose Abreu
2017-03-03 19:24 ` Alexey Brodkin
2017-03-03 19:24 ` Alexey Brodkin
2017-03-03 19:24 ` Alexey Brodkin
2017-03-06 10:48 ` Jose Abreu
2017-03-06 10:48 ` Jose Abreu
2017-03-06 10:48 ` Jose Abreu
2017-03-06 10:17 ` Daniel Vetter
2017-03-06 10:17 ` Daniel Vetter
2017-03-06 10:17 ` Daniel Vetter
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=1488547649.2940.5.camel@synopsys.com \
--to=alexey.brodkin@synopsys.com \
--cc=linux-snps-arc@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.