| Message ID | 20200421134410.30603-1-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Neil Armstrong |
| Headers | show |
| Series | drm/meson: add mode selection limits against specific SoC revisions | expand |
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. -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
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 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? Thank you! Martin
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 >
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); }
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> --- 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(-)