Add color management support for the crtc provided there are
enough dspps that can be allocated from the catalog
Kalyan Thota (3):
drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
drm/msm/disp/dpu1: add helper to know if display is builtin
drm/msm/disp/dpu1: add color management support for the crtc
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 5 ++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 33 +++++++++++++++++++++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 24 ++++++++++++++-------
5 files changed, 60 insertions(+), 13 deletions(-)
--
2.7.4
Since DRM encoder type for few encoders can be similar
(like eDP and DP), get the connector type for a given
encoder to differentiate between builtin and pluggable
displays.
Changes in v1:
- add connector type in the disp_info (Dmitry)
- add helper functions to know encoder type
- update commit text reflecting the change
Changes in v2:
- avoid hardcode of connector type for DSI as it may not be true (Dmitry)
- get the HPD information from encoder bridge
Changes in v3:
- use connector type instead of bridge ops in determining
connector (Dmitry)
Changes in v4:
- get type from the drm connector rather from bridge connector (Dmitry)
Signed-off-by: Kalyan Thota <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 ++++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c6817b..96db7fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -217,6 +217,32 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+bool dpu_encoder_is_builtin(struct drm_encoder *encoder)
+{
+ struct drm_connector *connector;
+ struct drm_connector_list_iter conn_iter;
+ struct drm_device *dev = encoder->dev;
+ int type = 0;
+
+ drm_connector_list_iter_begin(dev, &conn_iter);
+ drm_for_each_connector_iter(connector, &conn_iter) {
+ if (drm_connector_has_possible_encoder(connector, encoder)) {
+ type = connector->connector_type;
+ break;
+ }
+ }
+ drm_connector_list_iter_end(&conn_iter);
+
+ switch (type) {
+ case DRM_MODE_CONNECTOR_LVDS:
+ case DRM_MODE_CONNECTOR_eDP:
+ case DRM_MODE_CONNECTOR_DSI:
+ case DRM_MODE_CONNECTOR_DPI:
+ return true;
+ default:
+ return false;
+ }
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9e7236e..7f3d823 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -224,4 +224,10 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
*/
bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc);
+/**
+ * dpu_encoder_is_builtin - find if the encoder is of type builtin
+ * @drm_enc: Pointer to previously created drm encoder structure
+ */
+bool dpu_encoder_is_builtin(struct drm_encoder *drm_enc);
+
#endif /* __DPU_ENCODER_H__ */
--
2.7.4
Pin each crtc with one encoder. This arrangement will
disallow crtc switching between encoders and also will
facilitate to advertise certain features on crtc based
on encoder type.
Changes in v1:
- use drm_for_each_encoder macro while iterating through
encoder list (Dmitry)
Changes in v2:
- make sure no encoder miss to have a crtc (Dmitry)
- revisit various factors in deciding the crtc count
such as num_mixers, num_sspp (Dmitry)
Changes in v3:
- none
Changes in v4:
- use max_crtc_count instead of num_encoders in WARN (Dmitry)
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Kalyan Thota <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 7a5fabc..d967eef 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -747,6 +747,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
int primary_planes_idx = 0, cursor_planes_idx = 0, i, ret;
int max_crtc_count;
+
dev = dpu_kms->dev;
priv = dev->dev_private;
catalog = dpu_kms->catalog;
@@ -763,7 +764,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
drm_for_each_encoder(encoder, dev)
num_encoders++;
- max_crtc_count = min(catalog->mixer_count, num_encoders);
+ max_crtc_count = num_encoders;
/* Create the planes, keeping track of one primary/cursor per crtc */
for (i = 0; i < catalog->sspp_count; i++) {
@@ -795,22 +796,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
primary_planes[primary_planes_idx++] = plane;
}
- max_crtc_count = min(max_crtc_count, primary_planes_idx);
+ /*
+ * All the platforms should have at least 1 primary plane for a
+ * crtc. The below warn should help in setting up the catalog
+ */
+ WARN_ON(max_crtc_count > primary_planes_idx);
/* Create one CRTC per encoder */
- for (i = 0; i < max_crtc_count; i++) {
+ i = 0;
+ drm_for_each_encoder(encoder, dev) {
crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
if (IS_ERR(crtc)) {
ret = PTR_ERR(crtc);
return ret;
}
priv->crtcs[priv->num_crtcs++] = crtc;
+ encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
+ i++;
}
- /* All CRTCs are compatible with all encoders */
- drm_for_each_encoder(encoder, dev)
- encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
-
return 0;
}
--
2.7.4
On 21/11/2022 11:08, Kalyan Thota wrote:
> Since DRM encoder type for few encoders can be similar
> (like eDP and DP), get the connector type for a given
> encoder to differentiate between builtin and pluggable
> displays.
>
> Changes in v1:
> - add connector type in the disp_info (Dmitry)
> - add helper functions to know encoder type
> - update commit text reflecting the change
>
> Changes in v2:
> - avoid hardcode of connector type for DSI as it may not be true (Dmitry)
> - get the HPD information from encoder bridge
>
> Changes in v3:
> - use connector type instead of bridge ops in determining
> connector (Dmitry)
>
> Changes in v4:
> - get type from the drm connector rather from bridge connector (Dmitry)
>
> Signed-off-by: Kalyan Thota <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 ++++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++
> 2 files changed, 32 insertions(+)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Mon, 21 Nov 2022 01:08:12 -0800, Kalyan Thota wrote:
> Add color management support for the crtc provided there are
> enough dspps that can be allocated from the catalog
>
> Kalyan Thota (3):
> drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
> drm/msm/disp/dpu1: add helper to know if display is builtin
> drm/msm/disp/dpu1: add color management support for the crtc
>
> [...]
Applied, thanks!
[1/3] drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
https://gitlab.freedesktop.org/lumag/msm/-/commit/a4d6f8253645
[2/3] drm/msm/disp/dpu1: add helper to know if display is builtin
https://gitlab.freedesktop.org/lumag/msm/-/commit/4cb6b1eebb92
[3/3] drm/msm/disp/dpu1: add color management support for the crtc
https://gitlab.freedesktop.org/lumag/msm/-/commit/c48c475bd75a
Best regards,
--
Dmitry Baryshkov <[email protected]>
On 21/11/2022 11:08, Kalyan Thota wrote:
> Pin each crtc with one encoder. This arrangement will
> disallow crtc switching between encoders and also will
> facilitate to advertise certain features on crtc based
> on encoder type.
>
> Changes in v1:
> - use drm_for_each_encoder macro while iterating through
> encoder list (Dmitry)
>
> Changes in v2:
> - make sure no encoder miss to have a crtc (Dmitry)
> - revisit various factors in deciding the crtc count
> such as num_mixers, num_sspp (Dmitry)
>
> Changes in v3:
> - none
>
> Changes in v4:
> - use max_crtc_count instead of num_encoders in WARN (Dmitry)
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Kalyan Thota <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7a5fabc..d967eef 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -795,22 +796,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
> primary_planes[primary_planes_idx++] = plane;
> }
>
> - max_crtc_count = min(max_crtc_count, primary_planes_idx);
> + /*
> + * All the platforms should have at least 1 primary plane for a
> + * crtc. The below warn should help in setting up the catalog
> + */
> + WARN_ON(max_crtc_count > primary_planes_idx);
This change broke sc7180 support, see
https://gitlab.freedesktop.org/drm/msm/-/jobs/34395875
I suggest a quick fix of either disabling WB2 or switching one of cursor
SSPPs to a generic one.
>
> /* Create one CRTC per encoder */
> - for (i = 0; i < max_crtc_count; i++) {
> + i = 0;
> + drm_for_each_encoder(encoder, dev) {
> crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
> if (IS_ERR(crtc)) {
> ret = PTR_ERR(crtc);
> return ret;
> }
> priv->crtcs[priv->num_crtcs++] = crtc;
> + encoder->possible_crtcs = 1 << drm_crtc_index(crtc);
> + i++;
> }
>
> - /* All CRTCs are compatible with all encoders */
> - drm_for_each_encoder(encoder, dev)
> - encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
> -
> return 0;
> }
>
--
With best wishes
Dmitry
On 10/01/2023 01:43, Dmitry Baryshkov wrote:
>
> On Mon, 21 Nov 2022 01:08:12 -0800, Kalyan Thota wrote:
>> Add color management support for the crtc provided there are
>> enough dspps that can be allocated from the catalog
>>
>> Kalyan Thota (3):
>> drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
>> drm/msm/disp/dpu1: add helper to know if display is builtin
>> drm/msm/disp/dpu1: add color management support for the crtc
>>
>> [...]
>
> Applied, thanks!
>
> [1/3] drm/msm/disp/dpu1: pin 1 crtc to 1 encoder
> https://gitlab.freedesktop.org/lumag/msm/-/commit/a4d6f8253645
> [2/3] drm/msm/disp/dpu1: add helper to know if display is builtin
> https://gitlab.freedesktop.org/lumag/msm/-/commit/4cb6b1eebb92
> [3/3] drm/msm/disp/dpu1: add color management support for the crtc
> https://gitlab.freedesktop.org/lumag/msm/-/commit/c48c475bd75a
These patches break sc7180 in a bad way, as the SoC is short on SSPP
units. I'm going to carve these patches out and wait for better solution
for the color management issue.
>
> Best regards,
--
With best wishes
Dmitry