diff mbox series

[v2,1/7] drm/sun4i: tcon: Pass encoder instead of using panel for RGB setup

Message ID 20181101200045.6078-2-contact@paulk.fr (mailing list archive)
State Superseded
Headers show
Series BL035-RGB-002 3.5" LCD sunxi DRM support | expand

Commit Message

Paul Kocialkowski Nov. 1, 2018, 8 p.m. UTC
Features such as dithering and pixel data edge configuration currently
rely on the panel registered with the TCON driver. However, bridges are
also supported in addition panels.

Instead of retrieving the connector from the panel, pass the encoder
from the calling function, as is done for other interfaces.

The connector is then retrieved from the encoder with the dedicated
helper. Even in the case of bridges, the connector is registered with
the encoder from our driver and is accessible when iterating connectors.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

Comments

Maxime Ripard Nov. 5, 2018, 8:59 a.m. UTC | #1
Hi,

On Thu, Nov 01, 2018 at 09:00:39PM +0100, Paul Kocialkowski wrote:
> Features such as dithering and pixel data edge configuration currently
> rely on the panel registered with the TCON driver. However, bridges are
> also supported in addition panels.
> 
> Instead of retrieving the connector from the panel, pass the encoder
> from the calling function, as is done for other interfaces.
> 
> The connector is then retrieved from the encoder with the dedicated
> helper. Even in the case of bridges, the connector is registered with
> the encoder from our driver and is accessible when iterating connectors.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index f949287d926c..262ffb6b0f82 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -478,8 +478,11 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
>  }
>  
>  static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> +				     const struct drm_encoder *encoder,
>  				     const struct drm_display_mode *mode)
>  {
> +	struct drm_connector *connector = sun4i_tcon_get_connector(encoder);
> +	struct drm_display_info display_info = connector->display_info;
>  	unsigned int bp, hsync, vsync;
>  	u8 clk_delay;
>  	u32 val = 0;
> @@ -491,8 +494,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	sun4i_tcon0_mode_set_common(tcon, mode);
>  
>  	/* Set dithering if needed */
> -	if (tcon->panel)
> -		sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
> +	sun4i_tcon0_mode_set_dithering(tcon, connector);
>  
>  	/* Adjust clock delay */
>  	clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
> @@ -556,17 +558,11 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
>  	 * Following code is a way to avoid quirks all around TCON
>  	 * and DOTCLOCK drivers.
>  	 */
> -	if (tcon->panel) {
> -		struct drm_panel *panel = tcon->panel;
> -		struct drm_connector *connector = panel->connector;
> -		struct drm_display_info display_info = connector->display_info;
> -
> -		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> -			clk_set_phase(tcon->dclk, 240);
> +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> +		clk_set_phase(tcon->dclk, 240);
>  
> -		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> -			clk_set_phase(tcon->dclk, 0);
> -	}
> +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +		clk_set_phase(tcon->dclk, 0);

You're doing multiple things here: you're changing the function API,
and then moving the display info checks outside of the tcon->panel
condition.

You should do two separate patches for that.

Maxime
Paul Kocialkowski Nov. 5, 2018, 10:21 a.m. UTC | #2
Hi,

Le lundi 05 novembre 2018 à 09:59 +0100, Maxime Ripard a écrit :
> Hi,
> 
> On Thu, Nov 01, 2018 at 09:00:39PM +0100, Paul Kocialkowski wrote:
> > Features such as dithering and pixel data edge configuration currently
> > rely on the panel registered with the TCON driver. However, bridges are
> > also supported in addition panels.
> > 
> > Instead of retrieving the connector from the panel, pass the encoder
> > from the calling function, as is done for other interfaces.
> > 
> > The connector is then retrieved from the encoder with the dedicated
> > helper. Even in the case of bridges, the connector is registered with
> > the encoder from our driver and is accessible when iterating connectors.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++-------------
> >  1 file changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index f949287d926c..262ffb6b0f82 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -478,8 +478,11 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
> >  }
> >  
> >  static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> > +				     const struct drm_encoder *encoder,
> >  				     const struct drm_display_mode *mode)
> >  {
> > +	struct drm_connector *connector = sun4i_tcon_get_connector(encoder);
> > +	struct drm_display_info display_info = connector->display_info;
> >  	unsigned int bp, hsync, vsync;
> >  	u8 clk_delay;
> >  	u32 val = 0;
> > @@ -491,8 +494,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> >  	sun4i_tcon0_mode_set_common(tcon, mode);
> >  
> >  	/* Set dithering if needed */
> > -	if (tcon->panel)
> > -		sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
> > +	sun4i_tcon0_mode_set_dithering(tcon, connector);
> >  
> >  	/* Adjust clock delay */
> >  	clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
> > @@ -556,17 +558,11 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> >  	 * Following code is a way to avoid quirks all around TCON
> >  	 * and DOTCLOCK drivers.
> >  	 */
> > -	if (tcon->panel) {
> > -		struct drm_panel *panel = tcon->panel;
> > -		struct drm_connector *connector = panel->connector;
> > -		struct drm_display_info display_info = connector->display_info;
> > -
> > -		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> > -			clk_set_phase(tcon->dclk, 240);
> > +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
> > +		clk_set_phase(tcon->dclk, 240);
> >  
> > -		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > -			clk_set_phase(tcon->dclk, 0);
> > -	}
> > +	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> > +		clk_set_phase(tcon->dclk, 0);
> 
> You're doing multiple things here: you're changing the function API,
> and then moving the display info checks outside of the tcon->panel
> condition.
> 
> You should do two separate patches for that.

Thanks, I'll do that in the next revision then.

Cheers,

Paul
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index f949287d926c..262ffb6b0f82 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -478,8 +478,11 @@  static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
 }
 
 static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
+				     const struct drm_encoder *encoder,
 				     const struct drm_display_mode *mode)
 {
+	struct drm_connector *connector = sun4i_tcon_get_connector(encoder);
+	struct drm_display_info display_info = connector->display_info;
 	unsigned int bp, hsync, vsync;
 	u8 clk_delay;
 	u32 val = 0;
@@ -491,8 +494,7 @@  static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	sun4i_tcon0_mode_set_common(tcon, mode);
 
 	/* Set dithering if needed */
-	if (tcon->panel)
-		sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
+	sun4i_tcon0_mode_set_dithering(tcon, connector);
 
 	/* Adjust clock delay */
 	clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
@@ -556,17 +558,11 @@  static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	 * Following code is a way to avoid quirks all around TCON
 	 * and DOTCLOCK drivers.
 	 */
-	if (tcon->panel) {
-		struct drm_panel *panel = tcon->panel;
-		struct drm_connector *connector = panel->connector;
-		struct drm_display_info display_info = connector->display_info;
-
-		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
-			clk_set_phase(tcon->dclk, 240);
+	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)
+		clk_set_phase(tcon->dclk, 240);
 
-		if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
-			clk_set_phase(tcon->dclk, 0);
-	}
+	if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+		clk_set_phase(tcon->dclk, 0);
 
 	regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG,
 			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | SUN4I_TCON0_IO_POL_VSYNC_POSITIVE,
@@ -684,7 +680,7 @@  void sun4i_tcon_mode_set(struct sun4i_tcon *tcon,
 		sun4i_tcon0_mode_set_lvds(tcon, encoder, mode);
 		break;
 	case DRM_MODE_ENCODER_NONE:
-		sun4i_tcon0_mode_set_rgb(tcon, mode);
+		sun4i_tcon0_mode_set_rgb(tcon, encoder, mode);
 		sun4i_tcon_set_mux(tcon, 0, encoder);
 		break;
 	case DRM_MODE_ENCODER_TVDAC: