mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder
@ 2018-03-04 10:37 Giulio Benetti
  2018-03-04 10:44 ` Giulio Benetti
  2018-03-05  7:25 ` Maxime Ripard
  0 siblings, 2 replies; 3+ messages in thread
From: Giulio Benetti @ 2018-03-04 10:37 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel,
	linux-kernel, Giulio Benetti

mode_valid function must be connected to encoder, not connector.
Otherwise it doesn't get called by drm.

Move mode_valid function pointer to encoder helper functions,
changing its prototype according to encoder helper function pointer.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 drivers/gpu/drm/sun4i/sun4i_rgb.c | 102 +++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index b8da5a5..6539dcc 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -52,11 +52,59 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
 	return drm_panel_get_modes(tcon->panel);
 }
 
-static int sun4i_rgb_mode_valid(struct drm_connector *connector,
-				struct drm_display_mode *mode)
+static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
+	.get_modes	= sun4i_rgb_get_modes,
+};
+
+static void
+sun4i_rgb_connector_destroy(struct drm_connector *connector)
 {
 	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
 	struct sun4i_tcon *tcon = rgb->tcon;
+
+	drm_panel_detach(tcon->panel);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= sun4i_rgb_connector_destroy,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
+{
+	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
+	struct sun4i_tcon *tcon = rgb->tcon;
+
+	DRM_DEBUG_DRIVER("Enabling RGB output\n");
+
+	if (!IS_ERR(tcon->panel)) {
+		drm_panel_prepare(tcon->panel);
+		drm_panel_enable(tcon->panel);
+	}
+}
+
+static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
+	struct sun4i_tcon *tcon = rgb->tcon;
+
+	DRM_DEBUG_DRIVER("Disabling RGB output\n");
+
+	if (!IS_ERR(tcon->panel)) {
+		drm_panel_disable(tcon->panel);
+		drm_panel_unprepare(tcon->panel);
+	}
+}
+
+static enum drm_mode_status sun4i_rgb_encoder_mode_valid(struct drm_encoder *crtc,
+							 const struct drm_display_mode *mode)
+{
+	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(crtc);
+	struct sun4i_tcon *tcon = rgb->tcon;
 	u32 hsync = mode->hsync_end - mode->hsync_start;
 	u32 vsync = mode->vsync_end - mode->vsync_start;
 	unsigned long rate = mode->clock * 1000;
@@ -106,58 +154,10 @@ static int sun4i_rgb_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
-	.get_modes	= sun4i_rgb_get_modes,
-	.mode_valid	= sun4i_rgb_mode_valid,
-};
-
-static void
-sun4i_rgb_connector_destroy(struct drm_connector *connector)
-{
-	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
-	struct sun4i_tcon *tcon = rgb->tcon;
-
-	drm_panel_detach(tcon->panel);
-	drm_connector_cleanup(connector);
-}
-
-static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
-	.fill_modes		= drm_helper_probe_single_connector_modes,
-	.destroy		= sun4i_rgb_connector_destroy,
-	.reset			= drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
-};
-
-static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
-{
-	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
-	struct sun4i_tcon *tcon = rgb->tcon;
-
-	DRM_DEBUG_DRIVER("Enabling RGB output\n");
-
-	if (!IS_ERR(tcon->panel)) {
-		drm_panel_prepare(tcon->panel);
-		drm_panel_enable(tcon->panel);
-	}
-}
-
-static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
-{
-	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
-	struct sun4i_tcon *tcon = rgb->tcon;
-
-	DRM_DEBUG_DRIVER("Disabling RGB output\n");
-
-	if (!IS_ERR(tcon->panel)) {
-		drm_panel_disable(tcon->panel);
-		drm_panel_unprepare(tcon->panel);
-	}
-}
-
 static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = {
 	.disable	= sun4i_rgb_encoder_disable,
 	.enable		= sun4i_rgb_encoder_enable,
+	.mode_valid	= sun4i_rgb_encoder_mode_valid,
 };
 
 static void sun4i_rgb_enc_destroy(struct drm_encoder *encoder)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder
  2018-03-04 10:37 [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder Giulio Benetti
@ 2018-03-04 10:44 ` Giulio Benetti
  2018-03-05  7:25 ` Maxime Ripard
  1 sibling, 0 replies; 3+ messages in thread
From: Giulio Benetti @ 2018-03-04 10:44 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel

Hello everybody,

I wanted to move function sun4i_rgb_encoder_mode_valid from connector 
section to encoder section.
This is why it changes so many things, I don't know if it is the right 
way to do this.
Let me know.

Thanks

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

Il 04/03/2018 11:37, Giulio Benetti ha scritto:
> mode_valid function must be connected to encoder, not connector.
> Otherwise it doesn't get called by drm.
> 
> Move mode_valid function pointer to encoder helper functions,
> changing its prototype according to encoder helper function pointer.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>   drivers/gpu/drm/sun4i/sun4i_rgb.c | 102 +++++++++++++++++++-------------------
>   1 file changed, 51 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index b8da5a5..6539dcc 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -52,11 +52,59 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
>   	return drm_panel_get_modes(tcon->panel);
>   }
>   
> -static int sun4i_rgb_mode_valid(struct drm_connector *connector,
> -				struct drm_display_mode *mode)
> +static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
> +	.get_modes	= sun4i_rgb_get_modes,
> +};
> +
> +static void
> +sun4i_rgb_connector_destroy(struct drm_connector *connector)
>   {
>   	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
>   	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	drm_panel_detach(tcon->panel);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= sun4i_rgb_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> +	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	DRM_DEBUG_DRIVER("Enabling RGB output\n");
> +
> +	if (!IS_ERR(tcon->panel)) {
> +		drm_panel_prepare(tcon->panel);
> +		drm_panel_enable(tcon->panel);
> +	}
> +}
> +
> +static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> +	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	DRM_DEBUG_DRIVER("Disabling RGB output\n");
> +
> +	if (!IS_ERR(tcon->panel)) {
> +		drm_panel_disable(tcon->panel);
> +		drm_panel_unprepare(tcon->panel);
> +	}
> +}
> +
> +static enum drm_mode_status sun4i_rgb_encoder_mode_valid(struct drm_encoder *crtc,
> +							 const struct drm_display_mode *mode)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(crtc);
> +	struct sun4i_tcon *tcon = rgb->tcon;
>   	u32 hsync = mode->hsync_end - mode->hsync_start;
>   	u32 vsync = mode->vsync_end - mode->vsync_start;
>   	unsigned long rate = mode->clock * 1000;
> @@ -106,58 +154,10 @@ static int sun4i_rgb_mode_valid(struct drm_connector *connector,
>   	return MODE_OK;
>   }
>   
> -static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
> -	.get_modes	= sun4i_rgb_get_modes,
> -	.mode_valid	= sun4i_rgb_mode_valid,
> -};
> -
> -static void
> -sun4i_rgb_connector_destroy(struct drm_connector *connector)
> -{
> -	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	drm_panel_detach(tcon->panel);
> -	drm_connector_cleanup(connector);
> -}
> -
> -static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
> -	.fill_modes		= drm_helper_probe_single_connector_modes,
> -	.destroy		= sun4i_rgb_connector_destroy,
> -	.reset			= drm_atomic_helper_connector_reset,
> -	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
> -{
> -	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	DRM_DEBUG_DRIVER("Enabling RGB output\n");
> -
> -	if (!IS_ERR(tcon->panel)) {
> -		drm_panel_prepare(tcon->panel);
> -		drm_panel_enable(tcon->panel);
> -	}
> -}
> -
> -static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
> -{
> -	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	DRM_DEBUG_DRIVER("Disabling RGB output\n");
> -
> -	if (!IS_ERR(tcon->panel)) {
> -		drm_panel_disable(tcon->panel);
> -		drm_panel_unprepare(tcon->panel);
> -	}
> -}
> -
>   static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = {
>   	.disable	= sun4i_rgb_encoder_disable,
>   	.enable		= sun4i_rgb_encoder_enable,
> +	.mode_valid	= sun4i_rgb_encoder_mode_valid,
>   };
>   
>   static void sun4i_rgb_enc_destroy(struct drm_encoder *encoder)
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder
  2018-03-04 10:37 [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder Giulio Benetti
  2018-03-04 10:44 ` Giulio Benetti
@ 2018-03-05  7:25 ` Maxime Ripard
  1 sibling, 0 replies; 3+ messages in thread
From: Maxime Ripard @ 2018-03-05  7:25 UTC (permalink / raw)
  To: Giulio Benetti
  Cc: David Airlie, Chen-Yu Tsai, dri-devel, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5307 bytes --]



On Sun, Mar 04, 2018 at 11:37:50AM +0100, Giulio Benetti wrote:
> mode_valid function must be connected to encoder, not connector.
> Otherwise it doesn't get called by drm.

That's not true, or rather, that's a big oversimplification. It
doesn't get called by DRM if you have a bridge instead of a panel
attached to the encoder.

> Move mode_valid function pointer to encoder helper functions,
> changing its prototype according to encoder helper function pointer.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c | 102 +++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index b8da5a5..6539dcc 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -52,11 +52,59 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
>  	return drm_panel_get_modes(tcon->panel);
>  }
>  
> -static int sun4i_rgb_mode_valid(struct drm_connector *connector,
> -				struct drm_display_mode *mode)
> +static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
> +	.get_modes	= sun4i_rgb_get_modes,
> +};
> +
> +static void
> +sun4i_rgb_connector_destroy(struct drm_connector *connector)
>  {
>  	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
>  	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	drm_panel_detach(tcon->panel);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= sun4i_rgb_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> +	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	DRM_DEBUG_DRIVER("Enabling RGB output\n");
> +
> +	if (!IS_ERR(tcon->panel)) {
> +		drm_panel_prepare(tcon->panel);
> +		drm_panel_enable(tcon->panel);
> +	}
> +}
> +
> +static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> +	struct sun4i_tcon *tcon = rgb->tcon;
> +
> +	DRM_DEBUG_DRIVER("Disabling RGB output\n");
> +
> +	if (!IS_ERR(tcon->panel)) {
> +		drm_panel_disable(tcon->panel);
> +		drm_panel_unprepare(tcon->panel);
> +	}
> +}
> +
> +static enum drm_mode_status sun4i_rgb_encoder_mode_valid(struct drm_encoder *crtc,
> +							 const struct drm_display_mode *mode)
> +{
> +	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(crtc);
> +	struct sun4i_tcon *tcon = rgb->tcon;
>  	u32 hsync = mode->hsync_end - mode->hsync_start;
>  	u32 vsync = mode->vsync_end - mode->vsync_start;
>  	unsigned long rate = mode->clock * 1000;
> @@ -106,58 +154,10 @@ static int sun4i_rgb_mode_valid(struct drm_connector *connector,
>  	return MODE_OK;
>  }
>  
> -static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
> -	.get_modes	= sun4i_rgb_get_modes,
> -	.mode_valid	= sun4i_rgb_mode_valid,
> -};
> -
> -static void
> -sun4i_rgb_connector_destroy(struct drm_connector *connector)
> -{
> -	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	drm_panel_detach(tcon->panel);
> -	drm_connector_cleanup(connector);
> -}
> -
> -static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
> -	.fill_modes		= drm_helper_probe_single_connector_modes,
> -	.destroy		= sun4i_rgb_connector_destroy,
> -	.reset			= drm_atomic_helper_connector_reset,
> -	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
> -{
> -	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	DRM_DEBUG_DRIVER("Enabling RGB output\n");
> -
> -	if (!IS_ERR(tcon->panel)) {
> -		drm_panel_prepare(tcon->panel);
> -		drm_panel_enable(tcon->panel);
> -	}
> -}
> -
> -static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
> -{
> -	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
> -	struct sun4i_tcon *tcon = rgb->tcon;
> -
> -	DRM_DEBUG_DRIVER("Disabling RGB output\n");
> -
> -	if (!IS_ERR(tcon->panel)) {
> -		drm_panel_disable(tcon->panel);
> -		drm_panel_unprepare(tcon->panel);
> -	}
> -}
> -
>  static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = {
>  	.disable	= sun4i_rgb_encoder_disable,
>  	.enable		= sun4i_rgb_encoder_enable,
> +	.mode_valid	= sun4i_rgb_encoder_mode_valid,

The only thing you should be changing is that structure, the matching
connector structure, and the function prototype. This patch is way
bigger than it needs to be.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-03-05  7:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-04 10:37 [PATCH] drm/sun4i: move rgb mode_valid from connector to encoder Giulio Benetti
2018-03-04 10:44 ` Giulio Benetti
2018-03-05  7:25 ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®