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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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)
>
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 <[email protected]>
> ---
> 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