[v2,18/27] drm/rockchip: inno_hdmi: Subclass connector state
Commit Message
The data which is currently hold in hdmi_data should not be part of device
itself but of the connector state.
Introduce a connector state subclass and move the data from hdmi_data in
there.
Suggested-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Alex Bee <knaerzche@gmail.com>
---
changes in v2:
- new patch
drivers/gpu/drm/rockchip/inno_hdmi.c | 67 +++++++++++++++++++++-------
1 file changed, 50 insertions(+), 17 deletions(-)
Comments
Hi,
Thanks for working on that, it's much better now.
On Sat, Dec 16, 2023 at 05:26:29PM +0100, Alex Bee wrote:
> +static struct drm_connector_state *
> +inno_hdmi_connector_duplicate_state(struct drm_connector *connector)
> +{
> + struct inno_hdmi_connector_state *inno_conn_state;
> +
> + if (WARN_ON(!connector->state))
> + return NULL;
> +
> + inno_conn_state = kmemdup(to_inno_hdmi_conn_state(connector->state),
> + sizeof(*inno_conn_state), GFP_KERNEL);
> +
> + if (!inno_conn_state)
> + return NULL;
> +
> + __drm_atomic_helper_connector_duplicate_state(connector,
> + &inno_conn_state->base);
> +
> + return &inno_conn_state->base;
> +}
> +
> static const struct drm_connector_funcs inno_hdmi_connector_funcs = {
> .fill_modes = inno_hdmi_probe_single_connector_modes,
> .detect = inno_hdmi_connector_detect,
> .destroy = inno_hdmi_connector_destroy,
> .reset = drm_atomic_helper_connector_reset,
> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_duplicate_state = inno_hdmi_connector_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
You also need a custom reset and atomic_destroy_state implementations
Maxime
Hi,
Am 18.12.23 um 10:02 schrieb Maxime Ripard:
> Hi,
>
> Thanks for working on that, it's much better now.
>
> On Sat, Dec 16, 2023 at 05:26:29PM +0100, Alex Bee wrote:
>> +static struct drm_connector_state *
>> +inno_hdmi_connector_duplicate_state(struct drm_connector *connector)
>> +{
>> + struct inno_hdmi_connector_state *inno_conn_state;
>> +
>> + if (WARN_ON(!connector->state))
>> + return NULL;
>> +
>> + inno_conn_state = kmemdup(to_inno_hdmi_conn_state(connector->state),
>> + sizeof(*inno_conn_state), GFP_KERNEL);
>> +
>> + if (!inno_conn_state)
>> + return NULL;
>> +
>> + __drm_atomic_helper_connector_duplicate_state(connector,
>> + &inno_conn_state->base);
>> +
>> + return &inno_conn_state->base;
>> +}
>> +
>> static const struct drm_connector_funcs inno_hdmi_connector_funcs = {
>> .fill_modes = inno_hdmi_probe_single_connector_modes,
>> .detect = inno_hdmi_connector_detect,
>> .destroy = inno_hdmi_connector_destroy,
>> .reset = drm_atomic_helper_connector_reset,
>> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_duplicate_state = inno_hdmi_connector_duplicate_state,
>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> };
> You also need a custom reset and atomic_destroy_state implementations
Yea sure - thanks for the heads up.
I had this in multiple commits locally and thought I squashed them into
this one. Well, I obviously didn't :)
Alex
> Maxime
@@ -26,11 +26,6 @@
#include "inno_hdmi.h"
-struct hdmi_data_info {
- unsigned int enc_out_format;
- unsigned int colorimetry;
-};
-
struct inno_hdmi_i2c {
struct i2c_adapter adap;
@@ -54,8 +49,12 @@ struct inno_hdmi {
struct i2c_adapter *ddc;
unsigned int tmds_rate;
+};
- struct hdmi_data_info hdmi_data;
+struct inno_hdmi_connector_state {
+ struct drm_connector_state base;
+ unsigned int enc_out_format;
+ unsigned int colorimetry;
};
static struct inno_hdmi *encoder_to_inno_hdmi(struct drm_encoder *encoder)
@@ -70,6 +69,9 @@ static struct inno_hdmi *connector_to_inno_hdmi(struct drm_connector *connector)
return container_of(connector, struct inno_hdmi, connector);
}
+#define to_inno_hdmi_conn_state(_state) \
+ container_of_const(_state, struct inno_hdmi_connector_state, base)
+
enum {
CSC_RGB_0_255_TO_ITU601_16_235_8BIT,
CSC_RGB_0_255_TO_ITU709_16_235_8BIT,
@@ -248,6 +250,10 @@ static int inno_hdmi_upload_frame(struct inno_hdmi *hdmi,
static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
struct drm_display_mode *mode)
{
+ struct drm_connector *connector = &hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ struct inno_hdmi_connector_state *inno_conn_state =
+ to_inno_hdmi_conn_state(conn_state);
union hdmi_infoframe frame;
int rc;
@@ -259,9 +265,9 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
return rc;
}
- if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
+ if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444)
frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
- else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422)
+ else if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV422)
frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
else
frame.avi.colorspace = HDMI_COLORSPACE_RGB;
@@ -271,7 +277,10 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
{
- struct hdmi_data_info *data = &hdmi->hdmi_data;
+ struct drm_connector *connector = &hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ struct inno_hdmi_connector_state *inno_conn_state =
+ to_inno_hdmi_conn_state(conn_state);
int c0_c2_change = 0;
int csc_enable = 0;
int csc_mode = 0;
@@ -289,7 +298,7 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
v_VIDEO_INPUT_CSP(0);
hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL2, value);
- if (data->enc_out_format == HDMI_COLORSPACE_RGB) {
+ if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_RGB) {
value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
@@ -300,15 +309,15 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
return 0;
}
- if (data->colorimetry == HDMI_COLORIMETRY_ITU_601) {
- if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
+ if (inno_conn_state->colorimetry == HDMI_COLORIMETRY_ITU_601) {
+ if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444) {
csc_mode = CSC_RGB_0_255_TO_ITU601_16_235_8BIT;
auto_csc = AUTO_CSC_DISABLE;
c0_c2_change = C0_C2_CHANGE_DISABLE;
csc_enable = v_CSC_ENABLE;
}
} else {
- if (data->enc_out_format == HDMI_COLORSPACE_YUV444) {
+ if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444) {
csc_mode = CSC_RGB_0_255_TO_ITU709_16_235_8BIT;
auto_csc = AUTO_CSC_DISABLE;
c0_c2_change = C0_C2_CHANGE_DISABLE;
@@ -386,16 +395,20 @@ static int inno_hdmi_setup(struct inno_hdmi *hdmi,
{
struct drm_display_info *display = &hdmi->connector.display_info;
u8 vic = drm_match_cea_mode(mode);
+ struct drm_connector *connector = &hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ struct inno_hdmi_connector_state *inno_conn_state =
+ to_inno_hdmi_conn_state(conn_state);
- hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
+ inno_conn_state->enc_out_format = HDMI_COLORSPACE_RGB;
if (vic == 6 || vic == 7 ||
vic == 21 || vic == 22 ||
vic == 2 || vic == 3 ||
vic == 17 || vic == 18)
- hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_601;
+ inno_conn_state->colorimetry = HDMI_COLORIMETRY_ITU_601;
else
- hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
+ inno_conn_state->colorimetry = HDMI_COLORIMETRY_ITU_709;
/* Mute video and audio output */
hdmi_modb(hdmi, HDMI_AV_MUTE, m_AUDIO_MUTE | m_VIDEO_BLACK,
@@ -522,12 +535,32 @@ static void inno_hdmi_connector_destroy(struct drm_connector *connector)
drm_connector_cleanup(connector);
}
+static struct drm_connector_state *
+inno_hdmi_connector_duplicate_state(struct drm_connector *connector)
+{
+ struct inno_hdmi_connector_state *inno_conn_state;
+
+ if (WARN_ON(!connector->state))
+ return NULL;
+
+ inno_conn_state = kmemdup(to_inno_hdmi_conn_state(connector->state),
+ sizeof(*inno_conn_state), GFP_KERNEL);
+
+ if (!inno_conn_state)
+ return NULL;
+
+ __drm_atomic_helper_connector_duplicate_state(connector,
+ &inno_conn_state->base);
+
+ return &inno_conn_state->base;
+}
+
static const struct drm_connector_funcs inno_hdmi_connector_funcs = {
.fill_modes = inno_hdmi_probe_single_connector_modes,
.detect = inno_hdmi_connector_detect,
.destroy = inno_hdmi_connector_destroy,
.reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_duplicate_state = inno_hdmi_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};