Re: [PATCH v4 40/78] drm/vc4: hdmi: rework connectors and encoders
Hi Maxime
On Wed, 8 Jul 2020 at 18:43, Maxime Ripard wrote:
>
> the vc4_hdmi driver has some custom structures to hold the data it needs to
> associate with the drm_encoder and drm_connector structures.
>
> However, it allocates them separately from the vc4_hdmi structure which
> makes it more complicated than it needs to be.
>
> Move those structures to be contained by vc4_hdmi and update the code
> accordingly.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 87 ---
> drivers/gpu/drm/vc4/vc4_hdmi.h | 64 +-
> 2 files changed, 72 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index db79e0d88625..1e2214f24ed7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -191,20 +191,15 @@ static const struct drm_connector_helper_funcs
> vc4_hdmi_connector_helper_funcs =
> .get_modes = vc4_hdmi_connector_get_modes,
> };
>
> -static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
> -struct drm_encoder
> *encoder,
> -struct i2c_adapter *ddc)
> +static int vc4_hdmi_connector_init(struct drm_device *dev,
> + struct vc4_hdmi *vc4_hdmi,
> + struct i2c_adapter *ddc)
> {
> - struct drm_connector *connector;
> - struct vc4_hdmi_connector *hdmi_connector;
> + struct vc4_hdmi_connector *hdmi_connector = _hdmi->connector;
> + struct drm_connector *connector = _connector->base;
> + struct drm_encoder *encoder = _hdmi->encoder.base.base;
> int ret;
>
> - hdmi_connector = devm_kzalloc(dev->dev, sizeof(*hdmi_connector),
> - GFP_KERNEL);
> - if (!hdmi_connector)
> - return ERR_PTR(-ENOMEM);
> - connector = _connector->base;
> -
> hdmi_connector->encoder = encoder;
>
> drm_connector_init_with_ddc(dev, connector,
> @@ -216,7 +211,7 @@ static struct drm_connector
> *vc4_hdmi_connector_init(struct drm_device *dev,
> /* Create and attach TV margin props to this connector. */
> ret = drm_mode_create_tv_margin_properties(dev);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
>
> drm_connector_attach_tv_margin_properties(connector);
>
> @@ -228,7 +223,7 @@ static struct drm_connector
> *vc4_hdmi_connector_init(struct drm_device *dev,
>
> drm_connector_attach_encoder(connector, encoder);
>
> - return connector;
> + return 0;
> }
>
> static int vc4_hdmi_stop_packet(struct drm_encoder *encoder,
> @@ -298,21 +293,22 @@ static void vc4_hdmi_set_avi_infoframe(struct
> drm_encoder *encoder)
> struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
> struct vc4_dev *vc4 = encoder->dev->dev_private;
> struct vc4_hdmi *hdmi = vc4->hdmi;
> - struct drm_connector_state *cstate = hdmi->connector->state;
> + struct drm_connector *connector = >connector.base;
> + struct drm_connector_state *cstate = connector->state;
> struct drm_crtc *crtc = encoder->crtc;
> const struct drm_display_mode *mode = >state->adjusted_mode;
> union hdmi_infoframe frame;
> int ret;
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(,
> - hdmi->connector, mode);
> + connector, mode);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> }
>
> drm_hdmi_avi_infoframe_quant_range(,
> - hdmi->connector, mode,
> + connector, mode,
>vc4_encoder->limited_rgb_range ?
>HDMI_QUANTIZATION_RANGE_LIMITED :
>HDMI_QUANTIZATION_RANGE_FULL);
> @@ -628,7 +624,8 @@ static const struct drm_encoder_helper_funcs
> vc4_hdmi_encoder_helper_funcs = {
> /* HDMI audio codec callbacks */
> static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *hdmi)
> {
> - struct drm_device *drm = hdmi->encoder->dev;
> + struct drm_encoder *encoder = >encoder.base.base;
> + struct drm_device *drm = encoder->dev;
> struct vc4_dev *vc4 = to_vc4_dev(drm);
> u32 hsm_clock = clk_get_rate(hdmi->hsm_clock);
> unsigned long n, m;
> @@ -647,7 +644,7 @@ static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi
> *hdmi)
>
> static void vc4_hdmi_set_n_cts(struct vc4_hdmi *hdmi)
> {
> - struct drm_encoder *encoder = hdmi->encoder;
> + struct drm_encoder *encoder =
[PATCH v4 40/78] drm/vc4: hdmi: rework connectors and encoders
the vc4_hdmi driver has some custom structures to hold the data it needs to
associate with the drm_encoder and drm_connector structures.
However, it allocates them separately from the vc4_hdmi structure which
makes it more complicated than it needs to be.
Move those structures to be contained by vc4_hdmi and update the code
accordingly.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 87 ---
drivers/gpu/drm/vc4/vc4_hdmi.h | 64 +-
2 files changed, 72 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index db79e0d88625..1e2214f24ed7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -191,20 +191,15 @@ static const struct drm_connector_helper_funcs
vc4_hdmi_connector_helper_funcs =
.get_modes = vc4_hdmi_connector_get_modes,
};
-static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
-struct drm_encoder
*encoder,
-struct i2c_adapter *ddc)
+static int vc4_hdmi_connector_init(struct drm_device *dev,
+ struct vc4_hdmi *vc4_hdmi,
+ struct i2c_adapter *ddc)
{
- struct drm_connector *connector;
- struct vc4_hdmi_connector *hdmi_connector;
+ struct vc4_hdmi_connector *hdmi_connector = _hdmi->connector;
+ struct drm_connector *connector = _connector->base;
+ struct drm_encoder *encoder = _hdmi->encoder.base.base;
int ret;
- hdmi_connector = devm_kzalloc(dev->dev, sizeof(*hdmi_connector),
- GFP_KERNEL);
- if (!hdmi_connector)
- return ERR_PTR(-ENOMEM);
- connector = _connector->base;
-
hdmi_connector->encoder = encoder;
drm_connector_init_with_ddc(dev, connector,
@@ -216,7 +211,7 @@ static struct drm_connector *vc4_hdmi_connector_init(struct
drm_device *dev,
/* Create and attach TV margin props to this connector. */
ret = drm_mode_create_tv_margin_properties(dev);
if (ret)
- return ERR_PTR(ret);
+ return ret;
drm_connector_attach_tv_margin_properties(connector);
@@ -228,7 +223,7 @@ static struct drm_connector *vc4_hdmi_connector_init(struct
drm_device *dev,
drm_connector_attach_encoder(connector, encoder);
- return connector;
+ return 0;
}
static int vc4_hdmi_stop_packet(struct drm_encoder *encoder,
@@ -298,21 +293,22 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder
*encoder)
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_dev *vc4 = encoder->dev->dev_private;
struct vc4_hdmi *hdmi = vc4->hdmi;
- struct drm_connector_state *cstate = hdmi->connector->state;
+ struct drm_connector *connector = >connector.base;
+ struct drm_connector_state *cstate = connector->state;
struct drm_crtc *crtc = encoder->crtc;
const struct drm_display_mode *mode = >state->adjusted_mode;
union hdmi_infoframe frame;
int ret;
ret = drm_hdmi_avi_infoframe_from_display_mode(,
- hdmi->connector, mode);
+ connector, mode);
if (ret < 0) {
DRM_ERROR("couldn't fill AVI infoframe\n");
return;
}
drm_hdmi_avi_infoframe_quant_range(,
- hdmi->connector, mode,
+ connector, mode,
vc4_encoder->limited_rgb_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
HDMI_QUANTIZATION_RANGE_FULL);
@@ -628,7 +624,8 @@ static const struct drm_encoder_helper_funcs
vc4_hdmi_encoder_helper_funcs = {
/* HDMI audio codec callbacks */
static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *hdmi)
{
- struct drm_device *drm = hdmi->encoder->dev;
+ struct drm_encoder *encoder = >encoder.base.base;
+ struct drm_device *drm = encoder->dev;
struct vc4_dev *vc4 = to_vc4_dev(drm);
u32 hsm_clock = clk_get_rate(hdmi->hsm_clock);
unsigned long n, m;
@@ -647,7 +644,7 @@ static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi
*hdmi)
static void vc4_hdmi_set_n_cts(struct vc4_hdmi *hdmi)
{
- struct drm_encoder *encoder = hdmi->encoder;
+ struct drm_encoder *encoder = >encoder.base.base;
struct drm_crtc *crtc = encoder->crtc;
struct drm_device *drm = encoder->dev;
struct vc4_dev *vc4 = to_vc4_dev(drm);
@@ -685,7 +682,8 @@ static int vc4_hdmi_audio_startup(struct snd_pcm_substream
*substream,
struct
