Re: [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
Hi Neil,
Thank you for the patch.
On Fri, Oct 15, 2021 at 04:11:04PM +0200, Neil Armstrong wrote:
> This moves all the non-DW-HDMI code where it should be:
> an encoder in the drm/meson core driver.
>
> The bridge functions are copied as-is, except:
> - the encoder init uses the simple kms helper
> - the mode_set has been moved to atomic_enable()
> - debug prints are converted to dev_debg()
>
> For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR
> will be handled later.
>
> The meson dw-hdmi glue is slighly fixed to live without the
> encoder in the same driver.
>
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/meson/Makefile | 1 +
> drivers/gpu/drm/meson/meson_drv.c | 5 +
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 341 ++-
> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 375 +
> drivers/gpu/drm/meson/meson_encoder_hdmi.h | 12 +
> 5 files changed, 412 insertions(+), 322 deletions(-)
> create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.c
> create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.h
>
> diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
> index 28a519cdf66b..523fce45f16b 100644
> --- a/drivers/gpu/drm/meson/Makefile
> +++ b/drivers/gpu/drm/meson/Makefile
> @@ -2,6 +2,7 @@
> meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o
> meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o
> meson_overlay.o
> meson-drm-y += meson_rdma.o meson_osd_afbcd.o
> +meson-drm-y += meson_encoder_hdmi.o
>
> obj-$(CONFIG_DRM_MESON) += meson-drm.o
> obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
> diff --git a/drivers/gpu/drm/meson/meson_drv.c
> b/drivers/gpu/drm/meson/meson_drv.c
> index 97ebc07357bb..0978b440f336 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -32,6 +32,7 @@
> #include "meson_osd_afbcd.h"
> #include "meson_registers.h"
> #include "meson_venc_cvbs.h"
> +#include "meson_encoder_hdmi.h"
> #include "meson_viu.h"
> #include "meson_vpp.h"
> #include "meson_rdma.h"
> @@ -318,6 +319,10 @@ static int meson_drv_bind_master(struct device *dev,
> bool has_components)
> }
> }
>
> + ret = meson_encoder_hdmi_init(priv);
> + if (ret)
> + goto free_drm;
> +
> ret = meson_plane_create(priv);
> if (ret)
> goto free_drm;
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 0afbd1e70bfc..fb540a503efe 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -22,14 +22,11 @@
> #include
> #include
>
> -#include
> #include
>
> #include "meson_drv.h"
> #include "meson_dw_hdmi.h"
> #include "meson_registers.h"
> -#include "meson_vclk.h"
> -#include "meson_venc.h"
>
> #define DRIVER_NAME "meson-dw-hdmi"
> #define DRIVER_DESC "Amlogic Meson HDMI-TX DRM driver"
> @@ -135,8 +132,6 @@ struct meson_dw_hdmi_data {
> };
>
> struct meson_dw_hdmi {
> - struct drm_encoder encoder;
> - struct drm_bridge bridge;
> struct dw_hdmi_plat_data dw_plat_data;
> struct meson_drm *priv;
> struct device *dev;
> @@ -148,12 +143,8 @@ struct meson_dw_hdmi {
> struct regulator *hdmi_supply;
> u32 irq_stat;
> struct dw_hdmi *hdmi;
> - unsigned long output_bus_fmt;
> + struct drm_bridge *bridge;
> };
> -#define encoder_to_meson_dw_hdmi(x) \
> - container_of(x, struct meson_dw_hdmi, encoder)
> -#define bridge_to_meson_dw_hdmi(x) \
> - container_of(x, struct meson_dw_hdmi, bridge)
>
> static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
> const char *compat)
> @@ -295,14 +286,14 @@ static inline void dw_hdmi_dwc_write_bits(struct
> meson_dw_hdmi *dw_hdmi,
>
> /* Setup PHY bandwidth modes */
> static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
> - const struct drm_display_mode *mode)
> + const struct drm_display_mode *mode,
> + bool mode_is_420)
> {
> struct meson_drm *priv = dw_hdmi->priv;
> unsigned int pixel_clock = mode->clock;
>
> /* For 420, pixel clock is half unlike venc clock */
> - if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> - pixel_clock /= 2;
> + if (mode_is_420) pixel_clock /= 2;
>
> if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
> dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
> @@ -374,68 +365,25 @@ static inline void meson_dw_hdmi_phy_reset(struct
> meson_dw_hdmi *dw_hdmi)
> mdelay(2);
> }
>
> -static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
> - const struct drm_display_mode *mode)
> -{
> - struct meson_drm *priv =
Re: [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
Hi Neil, On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong wrote: > > This moves all the non-DW-HDMI code where it should be: > an encoder in the drm/meson core driver. > > The bridge functions are copied as-is, except: > - the encoder init uses the simple kms helper > - the mode_set has been moved to atomic_enable() > - debug prints are converted to dev_debg() > > For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR > will be handled later. > > The meson dw-hdmi glue is slighly fixed to live without the typo: slightly > encoder in the same driver. > > Signed-off-by: Neil Armstrong apart from that typo (please fix it up when applying the patch) this is some great work! it helps me a lot with HDMI support on the 32-bit SoCs with that said: Acked-by: Martin Blumenstingl
Re: [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
Hi Neil, On Fri, Oct 15, 2021 at 04:11:04PM +0200, Neil Armstrong wrote: > This moves all the non-DW-HDMI code where it should be: > an encoder in the drm/meson core driver. > > The bridge functions are copied as-is, except: > - the encoder init uses the simple kms helper > - the mode_set has been moved to atomic_enable() > - debug prints are converted to dev_debg() > > For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR > will be handled later. > > The meson dw-hdmi glue is slighly fixed to live without the > encoder in the same driver. > > Signed-off-by: Neil Armstrong Looks good, Acked-by: Sam Ravnborg With this I have covered this nice series. I am happy to see one more display driver that embraces fully the chained bridge approach. Sam
[PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
This moves all the non-DW-HDMI code where it should be:
an encoder in the drm/meson core driver.
The bridge functions are copied as-is, except:
- the encoder init uses the simple kms helper
- the mode_set has been moved to atomic_enable()
- debug prints are converted to dev_debg()
For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR
will be handled later.
The meson dw-hdmi glue is slighly fixed to live without the
encoder in the same driver.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/meson/Makefile | 1 +
drivers/gpu/drm/meson/meson_drv.c | 5 +
drivers/gpu/drm/meson/meson_dw_hdmi.c | 341 ++-
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 375 +
drivers/gpu/drm/meson/meson_encoder_hdmi.h | 12 +
5 files changed, 412 insertions(+), 322 deletions(-)
create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.c
create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.h
diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
index 28a519cdf66b..523fce45f16b 100644
--- a/drivers/gpu/drm/meson/Makefile
+++ b/drivers/gpu/drm/meson/Makefile
@@ -2,6 +2,7 @@
meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o
meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o
meson_overlay.o
meson-drm-y += meson_rdma.o meson_osd_afbcd.o
+meson-drm-y += meson_encoder_hdmi.o
obj-$(CONFIG_DRM_MESON) += meson-drm.o
obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
diff --git a/drivers/gpu/drm/meson/meson_drv.c
b/drivers/gpu/drm/meson/meson_drv.c
index 97ebc07357bb..0978b440f336 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -32,6 +32,7 @@
#include "meson_osd_afbcd.h"
#include "meson_registers.h"
#include "meson_venc_cvbs.h"
+#include "meson_encoder_hdmi.h"
#include "meson_viu.h"
#include "meson_vpp.h"
#include "meson_rdma.h"
@@ -318,6 +319,10 @@ static int meson_drv_bind_master(struct device *dev, bool
has_components)
}
}
+ ret = meson_encoder_hdmi_init(priv);
+ if (ret)
+ goto free_drm;
+
ret = meson_plane_create(priv);
if (ret)
goto free_drm;
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 0afbd1e70bfc..fb540a503efe 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -22,14 +22,11 @@
#include
#include
-#include
#include
#include "meson_drv.h"
#include "meson_dw_hdmi.h"
#include "meson_registers.h"
-#include "meson_vclk.h"
-#include "meson_venc.h"
#define DRIVER_NAME "meson-dw-hdmi"
#define DRIVER_DESC "Amlogic Meson HDMI-TX DRM driver"
@@ -135,8 +132,6 @@ struct meson_dw_hdmi_data {
};
struct meson_dw_hdmi {
- struct drm_encoder encoder;
- struct drm_bridge bridge;
struct dw_hdmi_plat_data dw_plat_data;
struct meson_drm *priv;
struct device *dev;
@@ -148,12 +143,8 @@ struct meson_dw_hdmi {
struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
- unsigned long output_bus_fmt;
+ struct drm_bridge *bridge;
};
-#define encoder_to_meson_dw_hdmi(x) \
- container_of(x, struct meson_dw_hdmi, encoder)
-#define bridge_to_meson_dw_hdmi(x) \
- container_of(x, struct meson_dw_hdmi, bridge)
static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
const char *compat)
@@ -295,14 +286,14 @@ static inline void dw_hdmi_dwc_write_bits(struct
meson_dw_hdmi *dw_hdmi,
/* Setup PHY bandwidth modes */
static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
- const struct drm_display_mode *mode)
+ const struct drm_display_mode *mode,
+ bool mode_is_420)
{
struct meson_drm *priv = dw_hdmi->priv;
unsigned int pixel_clock = mode->clock;
/* For 420, pixel clock is half unlike venc clock */
- if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
- pixel_clock /= 2;
+ if (mode_is_420) pixel_clock /= 2;
if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
@@ -374,68 +365,25 @@ static inline void meson_dw_hdmi_phy_reset(struct
meson_dw_hdmi *dw_hdmi)
mdelay(2);
}
-static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
-const struct drm_display_mode *mode)
-{
- struct meson_drm *priv = dw_hdmi->priv;
- int vic = drm_match_cea_mode(mode);
- unsigned int phy_freq;
- unsigned int vclk_freq;
- unsigned int venc_freq;
- unsigned int hdmi_freq;
-
- vclk_freq = mode->clock;
-
- /* For 420, pixel clock is half unlike venc clock */
- if
