* [PATCH 1/4] drm/sun4i: Move the panel pointer from the TCON to the encoders
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
@ 2019-02-26 14:25 ` Maxime Ripard
2019-03-07 13:29 ` Paul Kocialkowski
2019-02-26 14:25 ` [PATCH 2/4] drm/sun4i: rgb: Store the bridge pointer Maxime Ripard
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2019-02-26 14:25 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Paul Kocialkowski, dri-devel, Vasily Khoruzhick, Sean Paul,
linux-arm-kernel
The TCON driver used to need the panel pointer in order to configure the
tcon according to the various parameters of the panel. However, this has
evolved over time (especially to support bridges), and therefore the panel
pointer isn't needed anymore by the TCON driver.
Move that pointer to the LVDS and RGB encoders drivers.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 ++++++++++++-----------------
drivers/gpu/drm/sun4i/sun4i_rgb.c | 27 ++++++++++++---------------
drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 --
3 files changed, 24 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
index e7eb0d1e17be..5696f55db5cd 100644
--- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
+++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
@@ -20,7 +20,7 @@ struct sun4i_lvds {
struct drm_connector connector;
struct drm_encoder encoder;
- struct sun4i_tcon *tcon;
+ struct drm_panel *panel;
};
static inline struct sun4i_lvds *
@@ -41,9 +41,8 @@ static int sun4i_lvds_get_modes(struct drm_connector *connector)
{
struct sun4i_lvds *lvds =
drm_connector_to_sun4i_lvds(connector);
- struct sun4i_tcon *tcon = lvds->tcon;
- return drm_panel_get_modes(tcon->panel);
+ return drm_panel_get_modes(lvds->panel);
}
static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
@@ -54,9 +53,8 @@ static void
sun4i_lvds_connector_destroy(struct drm_connector *connector)
{
struct sun4i_lvds *lvds = drm_connector_to_sun4i_lvds(connector);
- struct sun4i_tcon *tcon = lvds->tcon;
- drm_panel_detach(tcon->panel);
+ drm_panel_detach(lvds->panel);
drm_connector_cleanup(connector);
}
@@ -71,26 +69,24 @@ static const struct drm_connector_funcs sun4i_lvds_con_funcs = {
static void sun4i_lvds_encoder_enable(struct drm_encoder *encoder)
{
struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(encoder);
- struct sun4i_tcon *tcon = lvds->tcon;
DRM_DEBUG_DRIVER("Enabling LVDS output\n");
- if (tcon->panel) {
- drm_panel_prepare(tcon->panel);
- drm_panel_enable(tcon->panel);
+ if (lvds->panel) {
+ drm_panel_prepare(lvds->panel);
+ drm_panel_enable(lvds->panel);
}
}
static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder)
{
struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(encoder);
- struct sun4i_tcon *tcon = lvds->tcon;
DRM_DEBUG_DRIVER("Disabling LVDS output\n");
- if (tcon->panel) {
- drm_panel_disable(tcon->panel);
- drm_panel_unprepare(tcon->panel);
+ if (lvds->panel) {
+ drm_panel_disable(lvds->panel);
+ drm_panel_unprepare(lvds->panel);
}
}
@@ -113,11 +109,10 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
lvds = devm_kzalloc(drm->dev, sizeof(*lvds), GFP_KERNEL);
if (!lvds)
return -ENOMEM;
- lvds->tcon = tcon;
encoder = &lvds->encoder;
ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
- &tcon->panel, &bridge);
+ &lvds->panel, &bridge);
if (ret) {
dev_info(drm->dev, "No panel or bridge found... LVDS output disabled\n");
return 0;
@@ -138,7 +133,7 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
/* The LVDS encoder can only work with the TCON channel 0 */
lvds->encoder.possible_crtcs = drm_crtc_mask(&tcon->crtc->crtc);
- if (tcon->panel) {
+ if (lvds->panel) {
drm_connector_helper_add(&lvds->connector,
&sun4i_lvds_con_helper_funcs);
ret = drm_connector_init(drm, &lvds->connector,
@@ -152,7 +147,7 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
drm_connector_attach_encoder(&lvds->connector,
&lvds->encoder);
- ret = drm_panel_attach(tcon->panel, &lvds->connector);
+ ret = drm_panel_attach(lvds->panel, &lvds->connector);
if (ret) {
dev_err(drm->dev, "Couldn't attach our panel\n");
goto err_cleanup_connector;
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index f4a22689eb54..d181ee399b03 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -27,6 +27,7 @@ struct sun4i_rgb {
struct drm_encoder encoder;
struct sun4i_tcon *tcon;
+ struct drm_panel *panel;
};
static inline struct sun4i_rgb *
@@ -47,9 +48,8 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
{
struct sun4i_rgb *rgb =
drm_connector_to_sun4i_rgb(connector);
- struct sun4i_tcon *tcon = rgb->tcon;
- return drm_panel_get_modes(tcon->panel);
+ return drm_panel_get_modes(rgb->panel);
}
static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
@@ -114,9 +114,8 @@ 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_panel_detach(rgb->panel);
drm_connector_cleanup(connector);
}
@@ -131,26 +130,24 @@ static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
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 (tcon->panel) {
- drm_panel_prepare(tcon->panel);
- drm_panel_enable(tcon->panel);
+ if (rgb->panel) {
+ drm_panel_prepare(rgb->panel);
+ drm_panel_enable(rgb->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 (tcon->panel) {
- drm_panel_disable(tcon->panel);
- drm_panel_unprepare(tcon->panel);
+ if (rgb->panel) {
+ drm_panel_disable(rgb->panel);
+ drm_panel_unprepare(rgb->panel);
}
}
@@ -183,7 +180,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
encoder = &rgb->encoder;
ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
- &tcon->panel, &bridge);
+ &rgb->panel, &bridge);
if (ret) {
dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
return 0;
@@ -204,7 +201,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
/* The RGB encoder can only work with the TCON channel 0 */
rgb->encoder.possible_crtcs = drm_crtc_mask(&tcon->crtc->crtc);
- if (tcon->panel) {
+ if (rgb->panel) {
drm_connector_helper_add(&rgb->connector,
&sun4i_rgb_con_helper_funcs);
ret = drm_connector_init(drm, &rgb->connector,
@@ -218,7 +215,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
drm_connector_attach_encoder(&rgb->connector,
&rgb->encoder);
- ret = drm_panel_attach(tcon->panel, &rgb->connector);
+ ret = drm_panel_attach(rgb->panel, &rgb->connector);
if (ret) {
dev_err(drm->dev, "Couldn't attach our panel\n");
goto err_cleanup_connector;
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index b5214d71610f..84cfb1952ff7 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -257,8 +257,6 @@ struct sun4i_tcon {
struct reset_control *lcd_rst;
struct reset_control *lvds_rst;
- struct drm_panel *panel;
-
/* Platform adjustments */
const struct sun4i_tcon_quirks *quirks;
--
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] drm/sun4i: Move the panel pointer from the TCON to the encoders
2019-02-26 14:25 ` [PATCH 1/4] drm/sun4i: Move the panel pointer from the TCON to the encoders Maxime Ripard
@ 2019-03-07 13:29 ` Paul Kocialkowski
0 siblings, 0 replies; 13+ messages in thread
From: Paul Kocialkowski @ 2019-03-07 13:29 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: Vasily Khoruzhick, Sean Paul, linux-arm-kernel, dri-devel
Hi,
On Tue, 2019-02-26 at 15:25 +0100, Maxime Ripard wrote:
> The TCON driver used to need the panel pointer in order to configure the
> tcon according to the various parameters of the panel. However, this has
> evolved over time (especially to support bridges), and therefore the panel
> pointer isn't needed anymore by the TCON driver.
>
> Move that pointer to the LVDS and RGB encoders drivers.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
That definitely makes good sense to me!
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cheers,
Paul
> ---
> drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 ++++++++++++-----------------
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 27 ++++++++++++---------------
> drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 --
> 3 files changed, 24 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> index e7eb0d1e17be..5696f55db5cd 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c
> @@ -20,7 +20,7 @@ struct sun4i_lvds {
> struct drm_connector connector;
> struct drm_encoder encoder;
>
> - struct sun4i_tcon *tcon;
> + struct drm_panel *panel;
> };
>
> static inline struct sun4i_lvds *
> @@ -41,9 +41,8 @@ static int sun4i_lvds_get_modes(struct drm_connector *connector)
> {
> struct sun4i_lvds *lvds =
> drm_connector_to_sun4i_lvds(connector);
> - struct sun4i_tcon *tcon = lvds->tcon;
>
> - return drm_panel_get_modes(tcon->panel);
> + return drm_panel_get_modes(lvds->panel);
> }
>
> static struct drm_connector_helper_funcs sun4i_lvds_con_helper_funcs = {
> @@ -54,9 +53,8 @@ static void
> sun4i_lvds_connector_destroy(struct drm_connector *connector)
> {
> struct sun4i_lvds *lvds = drm_connector_to_sun4i_lvds(connector);
> - struct sun4i_tcon *tcon = lvds->tcon;
>
> - drm_panel_detach(tcon->panel);
> + drm_panel_detach(lvds->panel);
> drm_connector_cleanup(connector);
> }
>
> @@ -71,26 +69,24 @@ static const struct drm_connector_funcs sun4i_lvds_con_funcs = {
> static void sun4i_lvds_encoder_enable(struct drm_encoder *encoder)
> {
> struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(encoder);
> - struct sun4i_tcon *tcon = lvds->tcon;
>
> DRM_DEBUG_DRIVER("Enabling LVDS output\n");
>
> - if (tcon->panel) {
> - drm_panel_prepare(tcon->panel);
> - drm_panel_enable(tcon->panel);
> + if (lvds->panel) {
> + drm_panel_prepare(lvds->panel);
> + drm_panel_enable(lvds->panel);
> }
> }
>
> static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder)
> {
> struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(encoder);
> - struct sun4i_tcon *tcon = lvds->tcon;
>
> DRM_DEBUG_DRIVER("Disabling LVDS output\n");
>
> - if (tcon->panel) {
> - drm_panel_disable(tcon->panel);
> - drm_panel_unprepare(tcon->panel);
> + if (lvds->panel) {
> + drm_panel_disable(lvds->panel);
> + drm_panel_unprepare(lvds->panel);
> }
> }
>
> @@ -113,11 +109,10 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> lvds = devm_kzalloc(drm->dev, sizeof(*lvds), GFP_KERNEL);
> if (!lvds)
> return -ENOMEM;
> - lvds->tcon = tcon;
> encoder = &lvds->encoder;
>
> ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
> - &tcon->panel, &bridge);
> + &lvds->panel, &bridge);
> if (ret) {
> dev_info(drm->dev, "No panel or bridge found... LVDS output disabled\n");
> return 0;
> @@ -138,7 +133,7 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> /* The LVDS encoder can only work with the TCON channel 0 */
> lvds->encoder.possible_crtcs = drm_crtc_mask(&tcon->crtc->crtc);
>
> - if (tcon->panel) {
> + if (lvds->panel) {
> drm_connector_helper_add(&lvds->connector,
> &sun4i_lvds_con_helper_funcs);
> ret = drm_connector_init(drm, &lvds->connector,
> @@ -152,7 +147,7 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> drm_connector_attach_encoder(&lvds->connector,
> &lvds->encoder);
>
> - ret = drm_panel_attach(tcon->panel, &lvds->connector);
> + ret = drm_panel_attach(lvds->panel, &lvds->connector);
> if (ret) {
> dev_err(drm->dev, "Couldn't attach our panel\n");
> goto err_cleanup_connector;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index f4a22689eb54..d181ee399b03 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -27,6 +27,7 @@ struct sun4i_rgb {
> struct drm_encoder encoder;
>
> struct sun4i_tcon *tcon;
> + struct drm_panel *panel;
> };
>
> static inline struct sun4i_rgb *
> @@ -47,9 +48,8 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
> {
> struct sun4i_rgb *rgb =
> drm_connector_to_sun4i_rgb(connector);
> - struct sun4i_tcon *tcon = rgb->tcon;
>
> - return drm_panel_get_modes(tcon->panel);
> + return drm_panel_get_modes(rgb->panel);
> }
>
> static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
> @@ -114,9 +114,8 @@ 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_panel_detach(rgb->panel);
> drm_connector_cleanup(connector);
> }
>
> @@ -131,26 +130,24 @@ static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
> 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 (tcon->panel) {
> - drm_panel_prepare(tcon->panel);
> - drm_panel_enable(tcon->panel);
> + if (rgb->panel) {
> + drm_panel_prepare(rgb->panel);
> + drm_panel_enable(rgb->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 (tcon->panel) {
> - drm_panel_disable(tcon->panel);
> - drm_panel_unprepare(tcon->panel);
> + if (rgb->panel) {
> + drm_panel_disable(rgb->panel);
> + drm_panel_unprepare(rgb->panel);
> }
> }
>
> @@ -183,7 +180,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> encoder = &rgb->encoder;
>
> ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
> - &tcon->panel, &bridge);
> + &rgb->panel, &bridge);
> if (ret) {
> dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
> return 0;
> @@ -204,7 +201,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> /* The RGB encoder can only work with the TCON channel 0 */
> rgb->encoder.possible_crtcs = drm_crtc_mask(&tcon->crtc->crtc);
>
> - if (tcon->panel) {
> + if (rgb->panel) {
> drm_connector_helper_add(&rgb->connector,
> &sun4i_rgb_con_helper_funcs);
> ret = drm_connector_init(drm, &rgb->connector,
> @@ -218,7 +215,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> drm_connector_attach_encoder(&rgb->connector,
> &rgb->encoder);
>
> - ret = drm_panel_attach(tcon->panel, &rgb->connector);
> + ret = drm_panel_attach(rgb->panel, &rgb->connector);
> if (ret) {
> dev_err(drm->dev, "Couldn't attach our panel\n");
> goto err_cleanup_connector;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index b5214d71610f..84cfb1952ff7 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -257,8 +257,6 @@ struct sun4i_tcon {
> struct reset_control *lcd_rst;
> struct reset_control *lvds_rst;
>
> - struct drm_panel *panel;
> -
> /* Platform adjustments */
> const struct sun4i_tcon_quirks *quirks;
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] drm/sun4i: rgb: Store the bridge pointer
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
2019-02-26 14:25 ` [PATCH 1/4] drm/sun4i: Move the panel pointer from the TCON to the encoders Maxime Ripard
@ 2019-02-26 14:25 ` Maxime Ripard
2019-03-07 13:29 ` Paul Kocialkowski
2019-02-26 14:25 ` [PATCH 3/4] drm/sun4i: Move rate variables to long long Maxime Ripard
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2019-02-26 14:25 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Paul Kocialkowski, dri-devel, Vasily Khoruzhick, Sean Paul,
linux-arm-kernel
We'll need the bridge pointer, if any, in the mode_valid callback in
addition to the init function. Store the pointer to the bridge in the
rgb private structure.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/gpu/drm/sun4i/sun4i_rgb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index d181ee399b03..8a43ed46af82 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -28,6 +28,7 @@ struct sun4i_rgb {
struct sun4i_tcon *tcon;
struct drm_panel *panel;
+ struct drm_bridge *bridge;
};
static inline struct sun4i_rgb *
@@ -169,7 +170,6 @@ static struct drm_encoder_funcs sun4i_rgb_enc_funcs = {
int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
{
struct drm_encoder *encoder;
- struct drm_bridge *bridge;
struct sun4i_rgb *rgb;
int ret;
@@ -180,7 +180,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
encoder = &rgb->encoder;
ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
- &rgb->panel, &bridge);
+ &rgb->panel, &rgb->bridge);
if (ret) {
dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
return 0;
@@ -222,8 +222,8 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
}
}
- if (bridge) {
- ret = drm_bridge_attach(encoder, bridge, NULL);
+ if (rgb->bridge) {
+ ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
if (ret) {
dev_err(drm->dev, "Couldn't attach our bridge\n");
goto err_cleanup_connector;
--
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] drm/sun4i: rgb: Store the bridge pointer
2019-02-26 14:25 ` [PATCH 2/4] drm/sun4i: rgb: Store the bridge pointer Maxime Ripard
@ 2019-03-07 13:29 ` Paul Kocialkowski
0 siblings, 0 replies; 13+ messages in thread
From: Paul Kocialkowski @ 2019-03-07 13:29 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: Vasily Khoruzhick, Sean Paul, linux-arm-kernel, dri-devel
Hi,
On Tue, 2019-02-26 at 15:25 +0100, Maxime Ripard wrote:
> We'll need the bridge pointer, if any, in the mode_valid callback in
> addition to the init function. Store the pointer to the bridge in the
> rgb private structure.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cheers,
Paul
> ---
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index d181ee399b03..8a43ed46af82 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -28,6 +28,7 @@ struct sun4i_rgb {
>
> struct sun4i_tcon *tcon;
> struct drm_panel *panel;
> + struct drm_bridge *bridge;
> };
>
> static inline struct sun4i_rgb *
> @@ -169,7 +170,6 @@ static struct drm_encoder_funcs sun4i_rgb_enc_funcs = {
> int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> {
> struct drm_encoder *encoder;
> - struct drm_bridge *bridge;
> struct sun4i_rgb *rgb;
> int ret;
>
> @@ -180,7 +180,7 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> encoder = &rgb->encoder;
>
> ret = drm_of_find_panel_or_bridge(tcon->dev->of_node, 1, 0,
> - &rgb->panel, &bridge);
> + &rgb->panel, &rgb->bridge);
> if (ret) {
> dev_info(drm->dev, "No panel or bridge found... RGB output disabled\n");
> return 0;
> @@ -222,8 +222,8 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
> }
> }
>
> - if (bridge) {
> - ret = drm_bridge_attach(encoder, bridge, NULL);
> + if (rgb->bridge) {
> + ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
> if (ret) {
> dev_err(drm->dev, "Couldn't attach our bridge\n");
> goto err_cleanup_connector;
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] drm/sun4i: Move rate variables to long long
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
2019-02-26 14:25 ` [PATCH 1/4] drm/sun4i: Move the panel pointer from the TCON to the encoders Maxime Ripard
2019-02-26 14:25 ` [PATCH 2/4] drm/sun4i: rgb: Store the bridge pointer Maxime Ripard
@ 2019-02-26 14:25 ` Maxime Ripard
2019-03-07 13:30 ` Paul Kocialkowski
2019-02-26 14:25 ` [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check Maxime Ripard
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2019-02-26 14:25 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Paul Kocialkowski, dri-devel, Vasily Khoruzhick, Sean Paul,
linux-arm-kernel
Our clock rate variables are getting pretty close to the LONG_MAX / ULONG_MAX
limit, especially since we will start doing arithmetic on it. Move those
types to unsigned long long to be sure we don't overflow their type.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/gpu/drm/sun4i/sun4i_rgb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index 8a43ed46af82..893b6e6d4d85 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -60,8 +60,8 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *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;
- long rounded_rate;
+ unsigned long long rate = mode->clock * 1000;
+ unsigned long long rounded_rate;
DRM_DEBUG_DRIVER("Validating modes...\n");
--
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] drm/sun4i: Move rate variables to long long
2019-02-26 14:25 ` [PATCH 3/4] drm/sun4i: Move rate variables to long long Maxime Ripard
@ 2019-03-07 13:30 ` Paul Kocialkowski
0 siblings, 0 replies; 13+ messages in thread
From: Paul Kocialkowski @ 2019-03-07 13:30 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: Vasily Khoruzhick, Sean Paul, linux-arm-kernel, dri-devel
Hi,
On Tue, 2019-02-26 at 15:25 +0100, Maxime Ripard wrote:
> Our clock rate variables are getting pretty close to the LONG_MAX / ULONG_MAX
> limit, especially since we will start doing arithmetic on it. Move those
> types to unsigned long long to be sure we don't overflow their type.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cheers,
Paul
> ---
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index 8a43ed46af82..893b6e6d4d85 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -60,8 +60,8 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *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;
> - long rounded_rate;
> + unsigned long long rate = mode->clock * 1000;
> + unsigned long long rounded_rate;
>
> DRM_DEBUG_DRIVER("Validating modes...\n");
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
` (2 preceding siblings ...)
2019-02-26 14:25 ` [PATCH 3/4] drm/sun4i: Move rate variables to long long Maxime Ripard
@ 2019-02-26 14:25 ` Maxime Ripard
2019-02-27 3:55 ` kbuild test robot
` (2 more replies)
2019-03-06 5:50 ` [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Vasily Khoruzhick
2019-03-07 13:50 ` Maxime Ripard
5 siblings, 3 replies; 13+ messages in thread
From: Maxime Ripard @ 2019-02-26 14:25 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Paul Kocialkowski, dri-devel, Vasily Khoruzhick, Sean Paul,
linux-arm-kernel
The current code, since commit bb43d40d7c83 ("drm/sun4i: rgb: Validate the
clock rate"), perform some validation on the pixel clock to filter out the
EDID modes provided by monitors (through bridges) that we wouldn't be able
to reach. For the usual modes, we're able to generate a perfect clock rate,
so a strict check was enough.
However, this had the side effect of preventing displays that would work
otherwise to operate properly, since we would pretty much never be able to
generate an exact rate for those displays, even though we would fall within
that panel tolerance.
This was also shown to happen for unusual modes exposed through EDIDs, for
example on eDP panels.
We can work around this by simplifying a bit the problem: no panels we've
encountered so far actually needed that check. All of them are tied to a
particular board when it is produced, and made to work with the Allwinner
BSP. That pretty much guarantees that we never have a pixel clock out of
reach.
On the other hand, the EDIDs modes that needed to be validated have always
been exposed through bridges.
Let's just use that metric to instead of validating all modes, only
validate modes when we have a bridge attached. It should be good enough for
now, while we still have room for improvements or refinements using the
display_timings structure for example for panels.
We also add a tolerance for EDID-based modes instead of doing a strict
check. This tolerance is of 0.5% which is the one advertised in the VESA
DVT and CVT specs. If that needed to be extended in the future, we can add
a custom module parameter to relax it a bit.
Fixes: bb43d40d7c83 ("drm/sun4i: rgb: Validate the clock rate")
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
drivers/gpu/drm/sun4i/sun4i_rgb.c | 37 ++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index 893b6e6d4d85..05beefe93989 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -53,6 +53,14 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
return drm_panel_get_modes(rgb->panel);
}
+/*
+ * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
+ * CVT spec reuses that tolerance in its examples, so it looks to be a
+ * good default tolerance for the EDID-based modes. Define it to 5 per
+ * mille to avoid floating point operations.
+ */
+DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
+
static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
const struct drm_display_mode *mode)
{
@@ -61,6 +69,7 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
u32 hsync = mode->hsync_end - mode->hsync_start;
u32 vsync = mode->vsync_end - mode->vsync_start;
unsigned long long rate = mode->clock * 1000;
+ unsigned long long lowest, highest;
unsigned long long rounded_rate;
DRM_DEBUG_DRIVER("Validating modes...\n");
@@ -93,15 +102,39 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
DRM_DEBUG_DRIVER("Vertical parameters OK\n");
+ /*
+ * TODO: We should use the struct display_timing if available
+ * and / or trying to stretch the timings within that
+ * tolerancy to take care of panels that we wouldn't be able
+ * to have a exact match for.
+ */
+ if (rgb->panel) {
+ DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
+ goto out;
+ }
+
+ /*
+ * That shouldn't ever happen unless something is really wrong, but it
+ * doesn't harm to check.
+ */
+ if (!rgb->bridge)
+ goto out;
+
tcon->dclk_min_div = 6;
tcon->dclk_max_div = 127;
rounded_rate = clk_round_rate(tcon->dclk, rate);
- if (rounded_rate < rate)
+
+ lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
+ do_div(lowest, 1000);
+ if (rounded_rate < lowest)
return MODE_CLOCK_LOW;
- if (rounded_rate > rate)
+ highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
+ do_div(highest, 1000);
+ if (rounded_rate > highest)
return MODE_CLOCK_HIGH;
+out:
DRM_DEBUG_DRIVER("Clock rate OK\n");
return MODE_OK;
--
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check
2019-02-26 14:25 ` [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check Maxime Ripard
@ 2019-02-27 3:55 ` kbuild test robot
2019-02-27 4:55 ` kbuild test robot
2019-03-07 13:36 ` Paul Kocialkowski
2 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2019-02-27 3:55 UTC (permalink / raw)
Cc: Vasily Khoruzhick, Maxime Ripard, dri-devel, Paul Kocialkowski,
Chen-Yu Tsai, Sean Paul, kbuild-all, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
Hi Maxime,
I love your patch! Yet something to improve:
[auto build test ERROR on ]
url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-rgb-Relax-the-pixel-clock-check/20190227-012757
base:
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:1: error: unknown type name 'DEFINE'; did you mean 'EMFILE'?
DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
^~~~~~
EMFILE
drivers/gpu//drm/sun4i/sun4i_rgb.c:62:37: error: expected '=', ',', ';', 'asm' or '__attribute__' before numeric constant
DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
^
drivers/gpu//drm/sun4i/sun4i_rgb.c:191:16: error: 'sun4i_rgb_mode_valid' undeclared here (not in a function); did you mean 'sun4i_rgb_con_funcs'?
.mode_valid = sun4i_rgb_mode_valid,
^~~~~~~~~~~~~~~~~~~~
sun4i_rgb_con_funcs
vim +62 drivers/gpu//drm/sun4i/sun4i_rgb.c
55
56 /*
57 * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
58 * CVT spec reuses that tolerance in its examples, so it looks to be a
59 * good default tolerance for the EDID-based modes. Define it to 5 per
60 * mille to avoid floating point operations.
61 */
> 62 DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
63
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63059 bytes --]
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check
2019-02-26 14:25 ` [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check Maxime Ripard
2019-02-27 3:55 ` kbuild test robot
@ 2019-02-27 4:55 ` kbuild test robot
2019-03-07 13:36 ` Paul Kocialkowski
2 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2019-02-27 4:55 UTC (permalink / raw)
Cc: Vasily Khoruzhick, Maxime Ripard, dri-devel, Paul Kocialkowski,
Chen-Yu Tsai, Sean Paul, kbuild-all, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 11258 bytes --]
Hi Maxime,
I love your patch! Yet something to improve:
[auto build test ERROR on ]
url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-rgb-Relax-the-pixel-clock-check/20190227-012757
base:
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:1: error: unknown type name 'DEFINE'; did you mean 'EPIPE'?
DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
^~~~~~
EPIPE
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:37: error: expected '=', ',', ';', 'asm' or '__attribute__' before numeric constant
DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
^
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:191:16: error: 'sun4i_rgb_mode_valid' undeclared here (not in a function); did you mean 'sun4i_rgb_con_funcs'?
.mode_valid = sun4i_rgb_mode_valid,
^~~~~~~~~~~~~~~~~~~~
sun4i_rgb_con_funcs
vim +191 drivers/gpu//drm/sun4i/sun4i_rgb.c
29e57fab9 Maxime Ripard 2015-10-29 55
3c3671edd Maxime Ripard 2019-02-26 56 /*
3c3671edd Maxime Ripard 2019-02-26 57 * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
3c3671edd Maxime Ripard 2019-02-26 58 * CVT spec reuses that tolerance in its examples, so it looks to be a
3c3671edd Maxime Ripard 2019-02-26 59 * good default tolerance for the EDID-based modes. Define it to 5 per
3c3671edd Maxime Ripard 2019-02-26 60 * mille to avoid floating point operations.
3c3671edd Maxime Ripard 2019-02-26 61 */
3c3671edd Maxime Ripard 2019-02-26 @62 DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
3c3671edd Maxime Ripard 2019-02-26 63
cde8b7548 Giulio Benetti 2018-03-13 64 static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
cde8b7548 Giulio Benetti 2018-03-13 65 const struct drm_display_mode *mode)
29e57fab9 Maxime Ripard 2015-10-29 66 {
cde8b7548 Giulio Benetti 2018-03-13 67 struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(crtc);
b9c8506cb Chen-Yu Tsai 2017-02-23 68 struct sun4i_tcon *tcon = rgb->tcon;
29e57fab9 Maxime Ripard 2015-10-29 69 u32 hsync = mode->hsync_end - mode->hsync_start;
29e57fab9 Maxime Ripard 2015-10-29 70 u32 vsync = mode->vsync_end - mode->vsync_start;
7659a82a3 Maxime Ripard 2019-02-26 71 unsigned long long rate = mode->clock * 1000;
3c3671edd Maxime Ripard 2019-02-26 72 unsigned long long lowest, highest;
7659a82a3 Maxime Ripard 2019-02-26 73 unsigned long long rounded_rate;
29e57fab9 Maxime Ripard 2015-10-29 74
29e57fab9 Maxime Ripard 2015-10-29 75 DRM_DEBUG_DRIVER("Validating modes...\n");
29e57fab9 Maxime Ripard 2015-10-29 76
29e57fab9 Maxime Ripard 2015-10-29 77 if (hsync < 1)
29e57fab9 Maxime Ripard 2015-10-29 78 return MODE_HSYNC_NARROW;
29e57fab9 Maxime Ripard 2015-10-29 79
29e57fab9 Maxime Ripard 2015-10-29 80 if (hsync > 0x3ff)
29e57fab9 Maxime Ripard 2015-10-29 81 return MODE_HSYNC_WIDE;
29e57fab9 Maxime Ripard 2015-10-29 82
29e57fab9 Maxime Ripard 2015-10-29 83 if ((mode->hdisplay < 1) || (mode->htotal < 1))
29e57fab9 Maxime Ripard 2015-10-29 84 return MODE_H_ILLEGAL;
29e57fab9 Maxime Ripard 2015-10-29 85
29e57fab9 Maxime Ripard 2015-10-29 86 if ((mode->hdisplay > 0x7ff) || (mode->htotal > 0xfff))
29e57fab9 Maxime Ripard 2015-10-29 87 return MODE_BAD_HVALUE;
29e57fab9 Maxime Ripard 2015-10-29 88
29e57fab9 Maxime Ripard 2015-10-29 89 DRM_DEBUG_DRIVER("Horizontal parameters OK\n");
29e57fab9 Maxime Ripard 2015-10-29 90
29e57fab9 Maxime Ripard 2015-10-29 91 if (vsync < 1)
29e57fab9 Maxime Ripard 2015-10-29 92 return MODE_VSYNC_NARROW;
29e57fab9 Maxime Ripard 2015-10-29 93
29e57fab9 Maxime Ripard 2015-10-29 94 if (vsync > 0x3ff)
29e57fab9 Maxime Ripard 2015-10-29 95 return MODE_VSYNC_WIDE;
29e57fab9 Maxime Ripard 2015-10-29 96
29e57fab9 Maxime Ripard 2015-10-29 97 if ((mode->vdisplay < 1) || (mode->vtotal < 1))
29e57fab9 Maxime Ripard 2015-10-29 98 return MODE_V_ILLEGAL;
29e57fab9 Maxime Ripard 2015-10-29 99
29e57fab9 Maxime Ripard 2015-10-29 100 if ((mode->vdisplay > 0x7ff) || (mode->vtotal > 0xfff))
29e57fab9 Maxime Ripard 2015-10-29 101 return MODE_BAD_VVALUE;
29e57fab9 Maxime Ripard 2015-10-29 102
29e57fab9 Maxime Ripard 2015-10-29 103 DRM_DEBUG_DRIVER("Vertical parameters OK\n");
29e57fab9 Maxime Ripard 2015-10-29 104
3c3671edd Maxime Ripard 2019-02-26 105 /*
3c3671edd Maxime Ripard 2019-02-26 106 * TODO: We should use the struct display_timing if available
3c3671edd Maxime Ripard 2019-02-26 107 * and / or trying to stretch the timings within that
3c3671edd Maxime Ripard 2019-02-26 108 * tolerancy to take care of panels that we wouldn't be able
3c3671edd Maxime Ripard 2019-02-26 109 * to have a exact match for.
3c3671edd Maxime Ripard 2019-02-26 110 */
3c3671edd Maxime Ripard 2019-02-26 111 if (rgb->panel) {
3c3671edd Maxime Ripard 2019-02-26 112 DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
3c3671edd Maxime Ripard 2019-02-26 113 goto out;
3c3671edd Maxime Ripard 2019-02-26 114 }
3c3671edd Maxime Ripard 2019-02-26 115
3c3671edd Maxime Ripard 2019-02-26 116 /*
3c3671edd Maxime Ripard 2019-02-26 117 * That shouldn't ever happen unless something is really wrong, but it
3c3671edd Maxime Ripard 2019-02-26 118 * doesn't harm to check.
3c3671edd Maxime Ripard 2019-02-26 119 */
3c3671edd Maxime Ripard 2019-02-26 120 if (!rgb->bridge)
3c3671edd Maxime Ripard 2019-02-26 121 goto out;
3c3671edd Maxime Ripard 2019-02-26 122
5af894bd2 Maxime Ripard 2018-02-21 123 tcon->dclk_min_div = 6;
5af894bd2 Maxime Ripard 2018-02-21 124 tcon->dclk_max_div = 127;
bb43d40d7 Maxime Ripard 2016-04-21 125 rounded_rate = clk_round_rate(tcon->dclk, rate);
3c3671edd Maxime Ripard 2019-02-26 126
3c3671edd Maxime Ripard 2019-02-26 127 lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
3c3671edd Maxime Ripard 2019-02-26 128 do_div(lowest, 1000);
3c3671edd Maxime Ripard 2019-02-26 129 if (rounded_rate < lowest)
bb43d40d7 Maxime Ripard 2016-04-21 130 return MODE_CLOCK_LOW;
bb43d40d7 Maxime Ripard 2016-04-21 131
3c3671edd Maxime Ripard 2019-02-26 132 highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
3c3671edd Maxime Ripard 2019-02-26 133 do_div(highest, 1000);
3c3671edd Maxime Ripard 2019-02-26 134 if (rounded_rate > highest)
bb43d40d7 Maxime Ripard 2016-04-21 135 return MODE_CLOCK_HIGH;
bb43d40d7 Maxime Ripard 2016-04-21 136
3c3671edd Maxime Ripard 2019-02-26 137 out:
bb43d40d7 Maxime Ripard 2016-04-21 138 DRM_DEBUG_DRIVER("Clock rate OK\n");
bb43d40d7 Maxime Ripard 2016-04-21 139
29e57fab9 Maxime Ripard 2015-10-29 140 return MODE_OK;
29e57fab9 Maxime Ripard 2015-10-29 141 }
29e57fab9 Maxime Ripard 2015-10-29 142
29e57fab9 Maxime Ripard 2015-10-29 143 static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
29e57fab9 Maxime Ripard 2015-10-29 144 .get_modes = sun4i_rgb_get_modes,
29e57fab9 Maxime Ripard 2015-10-29 145 };
29e57fab9 Maxime Ripard 2015-10-29 146
29e57fab9 Maxime Ripard 2015-10-29 147 static void
29e57fab9 Maxime Ripard 2015-10-29 148 sun4i_rgb_connector_destroy(struct drm_connector *connector)
29e57fab9 Maxime Ripard 2015-10-29 149 {
29e57fab9 Maxime Ripard 2015-10-29 150 struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
29e57fab9 Maxime Ripard 2015-10-29 151
b413dde1a Maxime Ripard 2019-02-26 152 drm_panel_detach(rgb->panel);
29e57fab9 Maxime Ripard 2015-10-29 153 drm_connector_cleanup(connector);
29e57fab9 Maxime Ripard 2015-10-29 154 }
29e57fab9 Maxime Ripard 2015-10-29 155
32b4d5756 Bhumika Goyal 2017-08-08 156 static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
29e57fab9 Maxime Ripard 2015-10-29 157 .fill_modes = drm_helper_probe_single_connector_modes,
29e57fab9 Maxime Ripard 2015-10-29 158 .destroy = sun4i_rgb_connector_destroy,
29e57fab9 Maxime Ripard 2015-10-29 159 .reset = drm_atomic_helper_connector_reset,
29e57fab9 Maxime Ripard 2015-10-29 160 .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
29e57fab9 Maxime Ripard 2015-10-29 161 .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
29e57fab9 Maxime Ripard 2015-10-29 162 };
29e57fab9 Maxime Ripard 2015-10-29 163
29e57fab9 Maxime Ripard 2015-10-29 164 static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
29e57fab9 Maxime Ripard 2015-10-29 165 {
29e57fab9 Maxime Ripard 2015-10-29 166 struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
29e57fab9 Maxime Ripard 2015-10-29 167
29e57fab9 Maxime Ripard 2015-10-29 168 DRM_DEBUG_DRIVER("Enabling RGB output\n");
29e57fab9 Maxime Ripard 2015-10-29 169
b413dde1a Maxime Ripard 2019-02-26 170 if (rgb->panel) {
b413dde1a Maxime Ripard 2019-02-26 171 drm_panel_prepare(rgb->panel);
b413dde1a Maxime Ripard 2019-02-26 172 drm_panel_enable(rgb->panel);
29e57fab9 Maxime Ripard 2015-10-29 173 }
45e88f994 Maxime Ripard 2017-10-17 174 }
29e57fab9 Maxime Ripard 2015-10-29 175
29e57fab9 Maxime Ripard 2015-10-29 176 static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
29e57fab9 Maxime Ripard 2015-10-29 177 {
29e57fab9 Maxime Ripard 2015-10-29 178 struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
29e57fab9 Maxime Ripard 2015-10-29 179
29e57fab9 Maxime Ripard 2015-10-29 180 DRM_DEBUG_DRIVER("Disabling RGB output\n");
29e57fab9 Maxime Ripard 2015-10-29 181
b413dde1a Maxime Ripard 2019-02-26 182 if (rgb->panel) {
b413dde1a Maxime Ripard 2019-02-26 183 drm_panel_disable(rgb->panel);
b413dde1a Maxime Ripard 2019-02-26 184 drm_panel_unprepare(rgb->panel);
4b3095025 Jonathan Liu 2016-08-30 185 }
45e88f994 Maxime Ripard 2017-10-17 186 }
29e57fab9 Maxime Ripard 2015-10-29 187
29e57fab9 Maxime Ripard 2015-10-29 188 static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = {
29e57fab9 Maxime Ripard 2015-10-29 189 .disable = sun4i_rgb_encoder_disable,
29e57fab9 Maxime Ripard 2015-10-29 190 .enable = sun4i_rgb_encoder_enable,
cde8b7548 Giulio Benetti 2018-03-13 @191 .mode_valid = sun4i_rgb_mode_valid,
29e57fab9 Maxime Ripard 2015-10-29 192 };
29e57fab9 Maxime Ripard 2015-10-29 193
:::::: The code at line 191 was first introduced by commit
:::::: cde8b7548272640437d2fa691ae211f940066f6b drm/sun4i: move rgb mode_valid from connector to encoder
:::::: TO: Giulio Benetti <giulio.benetti@micronovasrl.com>
:::::: CC: Maxime Ripard <maxime.ripard@bootlin.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68304 bytes --]
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check
2019-02-26 14:25 ` [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check Maxime Ripard
2019-02-27 3:55 ` kbuild test robot
2019-02-27 4:55 ` kbuild test robot
@ 2019-03-07 13:36 ` Paul Kocialkowski
2 siblings, 0 replies; 13+ messages in thread
From: Paul Kocialkowski @ 2019-03-07 13:36 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: Vasily Khoruzhick, Sean Paul, linux-arm-kernel, dri-devel
Hi,
On Tue, 2019-02-26 at 15:25 +0100, Maxime Ripard wrote:
> The current code, since commit bb43d40d7c83 ("drm/sun4i: rgb: Validate the
> clock rate"), perform some validation on the pixel clock to filter out the
> EDID modes provided by monitors (through bridges) that we wouldn't be able
> to reach. For the usual modes, we're able to generate a perfect clock rate,
> so a strict check was enough.
>
> However, this had the side effect of preventing displays that would work
> otherwise to operate properly, since we would pretty much never be able to
> generate an exact rate for those displays, even though we would fall within
> that panel tolerance.
>
> This was also shown to happen for unusual modes exposed through EDIDs, for
> example on eDP panels.
>
> We can work around this by simplifying a bit the problem: no panels we've
> encountered so far actually needed that check. All of them are tied to a
> particular board when it is produced, and made to work with the Allwinner
> BSP. That pretty much guarantees that we never have a pixel clock out of
> reach.
>
> On the other hand, the EDIDs modes that needed to be validated have always
> been exposed through bridges.
>
> Let's just use that metric to instead of validating all modes, only
> validate modes when we have a bridge attached. It should be good enough for
> now, while we still have room for improvements or refinements using the
> display_timings structure for example for panels.
>
> We also add a tolerance for EDID-based modes instead of doing a strict
> check. This tolerance is of 0.5% which is the one advertised in the VESA
> DVT and CVT specs. If that needed to be extended in the future, we can add
> a custom module parameter to relax it a bit.
>
> Fixes: bb43d40d7c83 ("drm/sun4i: rgb: Validate the clock rate")
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
With the define fixed and given the comment below, this is:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 37 ++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index 893b6e6d4d85..05beefe93989 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -53,6 +53,14 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
> return drm_panel_get_modes(rgb->panel);
> }
>
> +/*
> + * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
> + * CVT spec reuses that tolerance in its examples, so it looks to be a
> + * good default tolerance for the EDID-based modes. Define it to 5 per
> + * mille to avoid floating point operations.
> + */
> +DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
It could be nice to have some kind of unit made explicit in the define.
Maybe something like SUN4I_RGB_DOTCLOCK_TOLERANCE_PER_THOUSAND.
That's just a suggestion, feel free to take it or leave it :)
Cheers,
Paul
> +
> static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
> const struct drm_display_mode *mode)
> {
> @@ -61,6 +69,7 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
> u32 hsync = mode->hsync_end - mode->hsync_start;
> u32 vsync = mode->vsync_end - mode->vsync_start;
> unsigned long long rate = mode->clock * 1000;
> + unsigned long long lowest, highest;
> unsigned long long rounded_rate;
>
> DRM_DEBUG_DRIVER("Validating modes...\n");
> @@ -93,15 +102,39 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
>
> DRM_DEBUG_DRIVER("Vertical parameters OK\n");
>
> + /*
> + * TODO: We should use the struct display_timing if available
> + * and / or trying to stretch the timings within that
> + * tolerancy to take care of panels that we wouldn't be able
> + * to have a exact match for.
> + */
> + if (rgb->panel) {
> + DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
> + goto out;
> + }
> +
> + /*
> + * That shouldn't ever happen unless something is really wrong, but it
> + * doesn't harm to check.
> + */
> + if (!rgb->bridge)
> + goto out;
> +
> tcon->dclk_min_div = 6;
> tcon->dclk_max_div = 127;
> rounded_rate = clk_round_rate(tcon->dclk, rate);
> - if (rounded_rate < rate)
> +
> + lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
> + do_div(lowest, 1000);
> + if (rounded_rate < lowest)
> return MODE_CLOCK_LOW;
>
> - if (rounded_rate > rate)
> + highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
> + do_div(highest, 1000);
> + if (rounded_rate > highest)
> return MODE_CLOCK_HIGH;
>
> +out:
> DRM_DEBUG_DRIVER("Clock rate OK\n");
>
> return MODE_OK;
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
` (3 preceding siblings ...)
2019-02-26 14:25 ` [PATCH 4/4] drm/sun4i: rgb: Change the pixel clock validation check Maxime Ripard
@ 2019-03-06 5:50 ` Vasily Khoruzhick
2019-03-07 13:50 ` Maxime Ripard
5 siblings, 0 replies; 13+ messages in thread
From: Vasily Khoruzhick @ 2019-03-06 5:50 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, Paul Kocialkowski, Chen-Yu Tsai, Sean Paul, arm-linux
On Tue, Feb 26, 2019 at 6:25 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> This implements roughly what we discussed as part of the ANX6345 series to
> relax the pixel clock validation while still filtering out the modes we
> can't reach using the bridges.
>
> Let me know what you think,
> Maxime
With last patch fixed (s/DEFINE/#define), for whole series:
Tested-by: Vasily Khoruzhick <anarsoul@gmail.com> # tested on pinebook
> Maxime Ripard (4):
> drm/sun4i: Move the panel pointer from the TCON to the encoders
> drm/sun4i: rgb: Store the bridge pointer
> drm/sun4i: Move rate variables to long long
> drm/sun4i: rgb: Change the pixel clock validation check
>
> drivers/gpu/drm/sun4i/sun4i_lvds.c | 29 ++++--------
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 74 +++++++++++++++++++++----------
> drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 +-
> 3 files changed, 64 insertions(+), 41 deletions(-)
>
> base-commit: bfeffd155283772bbe78c6a05dec7c0128ee500c
> --
> git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check
2019-02-26 14:25 [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Maxime Ripard
` (4 preceding siblings ...)
2019-03-06 5:50 ` [PATCH 0/4] drm/sun4i: rgb: Relax the pixel clock check Vasily Khoruzhick
@ 2019-03-07 13:50 ` Maxime Ripard
5 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2019-03-07 13:50 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Paul Kocialkowski, dri-devel, Vasily Khoruzhick, Sean Paul,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]
On Tue, Feb 26, 2019 at 03:25:45PM +0100, Maxime Ripard wrote:
> Hi,
>
> This implements roughly what we discussed as part of the ANX6345 series to
> relax the pixel clock validation while still filtering out the modes we
> can't reach using the bridges.
>
> Let me know what you think,
> Maxime
Applied with Paul comments taken into account, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread