diff mbox

[v5,05/10] drm: arcpgu: Use crtc->mode_valid() callback

Message ID c3bcd69016c77f68a03ff3cb6b22ca6f90e930b0.1495720737.git.joabreu@synopsys.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jose Abreu May 25, 2017, 2:19 p.m. UTC
Now that we have a callback to check if crtc supports a given mode
we can use it in arcpgu so that we restrict the number of probbed
modes to the ones we can actually display.

This is specially useful because arcpgu crtc is responsible to set
a clock value in the commit() stage but unfortunatelly this clock
does not support all the needed ranges.

Also, remove the atomic_check() callback as mode_valid() callback
will be called before.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@linux.ie>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Changes v4->v5:
	- Change commit message to "arcpgu" (Alexey)
Changes v3->v4:
	- Do not use aux function (Laurent)

---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Neil Armstrong May 30, 2017, 7:28 a.m. UTC | #1
On 05/25/2017 04:19 PM, Jose Abreu wrote:
> Now that we have a callback to check if crtc supports a given mode
> we can use it in arcpgu so that we restrict the number of probbed
> modes to the ones we can actually display.
> 
> This is specially useful because arcpgu crtc is responsible to set
> a clock value in the commit() stage but unfortunatelly this clock
> does not support all the needed ranges.
> 
> Also, remove the atomic_check() callback as mode_valid() callback
> will be called before.
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@linux.ie>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Changes v4->v5:
> 	- Change commit message to "arcpgu" (Alexey)
> Changes v3->v4:
> 	- Do not use aux function (Laurent)
> 
> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ad9a959..99fbdae 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -64,6 +64,19 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };
>  
> +enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
> +					     const struct drm_display_mode *mode)
> +{
> +	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> +	long rate, clk_rate = mode->clock * 1000;
> +
> +	rate = clk_round_rate(arcpgu->clk, clk_rate);
> +	if (rate != clk_rate)
> +		return MODE_NOCLOCK;
> +
> +	return MODE_OK;
> +}
> +
>  static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  {
>  	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> @@ -129,20 +142,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)
>  {
> @@ -158,6 +157,7 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
> +	.mode_valid	= arc_pgu_crtc_mode_valid,
>  	.mode_set	= drm_helper_crtc_mode_set,
>  	.mode_set_base	= drm_helper_crtc_mode_set_base,
>  	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
> @@ -165,7 +165,6 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
>  	.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,
>  };
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Jose Abreu June 21, 2017, 9:38 a.m. UTC | #2
Hi Daniel, Alexey,


On 25-05-2017 15:19, Jose Abreu wrote:
> Now that we have a callback to check if crtc supports a given mode
> we can use it in arcpgu so that we restrict the number of probbed
> modes to the ones we can actually display.
>
> This is specially useful because arcpgu crtc is responsible to set
> a clock value in the commit() stage but unfortunatelly this clock
> does not support all the needed ranges.
>
> Also, remove the atomic_check() callback as mode_valid() callback
> will be called before.
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>

@Daniel: I know that arcpgu is not under drm-misc but could you
apply this into drm-misc-next? Alexey (the arcpgu maintainer)
already gave the reviewed-by and this is also "Reviewed-by: Neil
Armstrong <narmstrong@baylibre.com>". It was also tested-by: me.

@Alexey: Is this ok for you?

I need this because I have a pending patch for accepting clock
variations instead of always failing when clk_round_rate() does
not return the expected rate.

Best regards,
Jose Miguel Abreu

> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@linux.ie>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Changes v4->v5:
> 	- Change commit message to "arcpgu" (Alexey)
> Changes v3->v4:
> 	- Do not use aux function (Laurent)
>
> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ad9a959..99fbdae 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -64,6 +64,19 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };
>  
> +enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
> +					     const struct drm_display_mode *mode)
> +{
> +	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> +	long rate, clk_rate = mode->clock * 1000;
> +
> +	rate = clk_round_rate(arcpgu->clk, clk_rate);
> +	if (rate != clk_rate)
> +		return MODE_NOCLOCK;
> +
> +	return MODE_OK;
> +}
> +
>  static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  {
>  	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> @@ -129,20 +142,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)
>  {
> @@ -158,6 +157,7 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
> +	.mode_valid	= arc_pgu_crtc_mode_valid,
>  	.mode_set	= drm_helper_crtc_mode_set,
>  	.mode_set_base	= drm_helper_crtc_mode_set_base,
>  	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
> @@ -165,7 +165,6 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
>  	.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,
>  };
>
Daniel Vetter June 22, 2017, 8:46 a.m. UTC | #3
On Wed, Jun 21, 2017 at 10:38:43AM +0100, Jose Abreu wrote:
> Hi Daniel, Alexey,
> 
> 
> On 25-05-2017 15:19, Jose Abreu wrote:
> > Now that we have a callback to check if crtc supports a given mode
> > we can use it in arcpgu so that we restrict the number of probbed
> > modes to the ones we can actually display.
> >
> > This is specially useful because arcpgu crtc is responsible to set
> > a clock value in the commit() stage but unfortunatelly this clock
> > does not support all the needed ranges.
> >
> > Also, remove the atomic_check() callback as mode_valid() callback
> > will be called before.
> >
> > Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> > Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>
> 
> @Daniel: I know that arcpgu is not under drm-misc but could you
> apply this into drm-misc-next? Alexey (the arcpgu maintainer)
> already gave the reviewed-by and this is also "Reviewed-by: Neil
> Armstrong <narmstrong@baylibre.com>". It was also tested-by: me.
> 
> @Alexey: Is this ok for you?
> 
> I need this because I have a pending patch for accepting clock
> variations instead of always failing when clk_round_rate() does
> not return the expected rate.

Alexey reviewed it already, and Dave acked it for merging through
drm-misc, so applied.

Thanks, Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a959..99fbdae 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -64,6 +64,19 @@  static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
+enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
+					     const struct drm_display_mode *mode)
+{
+	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
+	long rate, clk_rate = mode->clock * 1000;
+
+	rate = clk_round_rate(arcpgu->clk, clk_rate);
+	if (rate != clk_rate)
+		return MODE_NOCLOCK;
+
+	return MODE_OK;
+}
+
 static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
@@ -129,20 +142,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)
 {
@@ -158,6 +157,7 @@  static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
+	.mode_valid	= arc_pgu_crtc_mode_valid,
 	.mode_set	= drm_helper_crtc_mode_set,
 	.mode_set_base	= drm_helper_crtc_mode_set_base,
 	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
@@ -165,7 +165,6 @@  static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
 	.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,
 };