On 21/04/2020 15:59, Daniel Vetter wrote:

On Tue, Apr 21, 2020 at 03:44:10PM +0200, Neil Armstrong wrote:

...

The Amlogic S805X/Y uses the same die as the S905X, but with more limited graphics capabilities.

This adds a soc version detection adding specific limitations on the HDMI mode selections.

Here, we limit to HDMI 1.3a max HDMI PHY clock frequency.

Signed-off-by: Neil Armstrong narmstrong@baylibre.com

Just a drive-by, but the code organization between the dw-hdmi bridge and the driver looks pretty terribly and really leaky. Can't we do better? Either by fixing the dw-hdmi bridge abstraction to actually abstract something, or by givin up the dw-hdmi is a bridge and convert it to some helper to implement a drm_encoder. Current status just doesn't make too much sense to me.

Yep, the encoder part should be moved out of the dw-hdmi glue driver for sure. I'll a draft something, but it won't really affect this patch.

Neil

-Daniel

...

---

drivers/gpu/drm/meson/meson_drv.c | 29 ++++++++++++++++++++++++++- drivers/gpu/drm/meson/meson_drv.h | 6 ++++++ drivers/gpu/drm/meson/meson_dw_hdmi.c | 7 +++++++ 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 6f29fab79952..621f6de0f076 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -11,6 +11,7 @@ #include <linux/component.h> #include <linux/module.h> #include <linux/of_graph.h> +#include <linux/sys_soc.h> #include <linux/platform_device.h> #include <linux/soc/amlogic/meson-canvas.h>

@@ -183,6 +184,24 @@ static void meson_remove_framebuffers(void) kfree(ap); }

+struct meson_drm_soc_attr {

• struct meson_drm_soc_limits limits;
• const struct soc_device_attribute *attrs;

+};

+static const struct meson_drm_soc_attr meson_drm_soc_attrs[] = {

• /* S805X/S805Y HDMI PLL won't lock for HDMI PHY freq > 1,65GHz */
• {

• .limits = {
  

• 	.max_hdmi_phy_freq = 1650000,
  

• },
  

• .attrs = (const struct soc_device_attribute []) {
  

• 	{ .soc_id = "GXL (S805*)", },
  

• 	{ /* sentinel */ },
  

• }
  

• },

+};

static int meson_drv_bind_master(struct device *dev, bool has_components) { struct platform_device *pdev = to_platform_device(dev); @@ -191,7 +210,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) struct drm_device *drm; struct resource *res; void __iomem *regs;

• int ret;

• int ret, i;

  /* Checks if an output connector is available */ if (!meson_vpu_has_available_connectors(dev)) {

@@ -281,6 +300,14 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (ret) goto free_drm;

• /* Assign limits per soc revision/package */
• for (i = 0 ; i < ARRAY_SIZE(meson_drm_soc_attrs) ; ++i) {

• if (soc_device_match(meson_drm_soc_attrs[i].attrs)) {
  

• 	priv->limits = &meson_drm_soc_attrs[i].limits;
  

• 	break;
  

• }
  

• }
• /* Remove early framebuffers (ie. simplefb) */ meson_remove_framebuffers();

diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h index 04fdf3826643..5b23704a80d6 100644 --- a/drivers/gpu/drm/meson/meson_drv.h +++ b/drivers/gpu/drm/meson/meson_drv.h @@ -30,6 +30,10 @@ struct meson_drm_match_data { struct meson_afbcd_ops *afbcd_ops; };

+struct meson_drm_soc_limits {

• unsigned int max_hdmi_phy_freq;

+};

struct meson_drm { struct device *dev; enum vpu_compatible compat; @@ -48,6 +52,8 @@ struct meson_drm { struct drm_plane *primary_plane; struct drm_plane *overlay_plane;

• const struct meson_drm_soc_limits *limits;
• /* Components Data */ struct { bool osd1_enabled;

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c index e8c94915a4fc..dc3d5122475a 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c @@ -695,6 +695,13 @@ dw_hdmi_mode_valid(struct drm_connector *connector, dev_dbg(connector->dev->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n", __func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);

• /* Check against soc revision/package limits */
• if (priv->limits) {

• if (priv->limits->max_hdmi_phy_freq &&
  

•     phy_freq > priv->limits->max_hdmi_phy_freq)
  

• 	return MODE_CLOCK_HIGH;
  

• }
• return meson_vclk_vic_supported_freq(phy_freq, vclk_freq);

}

-- 2.22.0

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

Hi,

On 22/04/2020 23:12, Martin Blumenstingl wrote:

Hi Neil,

On Tue, Apr 21, 2020 at 3:44 PM Neil Armstrong narmstrong@baylibre.com wrote: [...]

...

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c index e8c94915a4fc..dc3d5122475a 100644 --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c @@ -695,6 +695,13 @@ dw_hdmi_mode_valid(struct drm_connector *connector, dev_dbg(connector->dev->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n", __func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);

•   /* Check against soc revision/package limits */
  

•   if (priv->limits) {
  

•           if (priv->limits->max_hdmi_phy_freq &&
  

•               phy_freq > priv->limits->max_hdmi_phy_freq)
  

•                   return MODE_CLOCK_HIGH;
  

•   }
  

I think that this will also be useful for the 32-bit SoCs as well. is there a chance you can move it to meson_vclk_vic_supported_freq (called right below), where all the existing frequency limit checks are already?

It would need to add priv to meson_vclk_vic_supported_freq(), but indeed, would be cleaner.

And the meson_vclk_dmt_supported_freq() would also need this test aswell.

I'll resend with these fixed.

Neil

Thank you! Martin