Re: [Freedreno] [PATCH 3/3] drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set
On 8/17/2023 12:27 PM, Abhinav Kumar wrote: On 8/17/2023 11:50 AM, Dmitry Baryshkov wrote: On 08/08/2023 02:49, Abhinav Kumar wrote: On 6/4/2023 7:45 AM, Dmitry Baryshkov wrote: The atomic_mode_set() callback only sets the phys_enc's IRQ data. As the INTF and WB are statically allocated to each encoder/phys_enc, drop the atomic_mode_set callback and set the IRQs during encoder init. For the CMD panel usecase some of IRQ indexes depend on the selected resources. Move setting them to the irq_enable() callback. The irq_enable() callback is called from the dpu_encoder_virt_atomic_enable() after the phys layer's enable. Thats late. So lets consider the case where command mode panel's clock is powered from bootloader (quite common). Now, as soon as the tearcheck is configured and interface is ON from the phys's enable(), nothing prevents / should prevent the interrupt from firing. Please note that interrupt callbacks are also registered from the irq_control(). There is no way the interrupt can fire beforehand (and the call chain is dpu_encoder_virt_atomic_enable() -> dpu_encoder_resource_control() -> _dpu_encoder_resource_control_helper() -> _dpu_encoder_irq_control() -> irq_control(). If we ever want to move irq_control() to be called before phys's enable() callbacks, this will be a separate patchset, involving refactoring of dpu_encoder_resource_control(). Ack, I will rebase my patches on top of this and re-test it. Then will give my R-b, tested-by tags by Monday. Sorry for the delay on this. Reviewed-by: Abhinav Kumar Tested-by: Abhinav Kumar # sc7280 Made sure that sc7280 boots up fine and kms_writeback works
Re: [Freedreno] [PATCH 2/3] drm/msm/dpu: split _dpu_encoder_resource_control_helper()
On 6/4/2023 7:45 AM, Dmitry Baryshkov wrote:
Follow the _dpu_encoder_irq_control() change and split the
_dpu_encoder_resource_control_helper() into enable and disable parts.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 45 +
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7c131c5cbe71..cc61f0cf059d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -757,8 +757,7 @@ static void _dpu_encoder_irq_disable(struct drm_encoder
*drm_enc)
}
}
-static void _dpu_encoder_resource_control_helper(struct drm_encoder *drm_enc,
- bool enable)
+static void _dpu_encoder_resource_enable(struct drm_encoder *drm_enc)
{
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
@@ -768,28 +767,42 @@ static void _dpu_encoder_resource_control_helper(struct
drm_encoder *drm_enc,
priv = drm_enc->dev->dev_private;
dpu_kms = to_dpu_kms(priv->kms);
- trace_dpu_enc_rc_helper(DRMID(drm_enc), enable);
+ trace_dpu_enc_rc_helper(DRMID(drm_enc), true);
same question about trace here.
if (!dpu_enc->cur_master) {
DPU_ERROR("encoder master not set\n");
return;
}
- if (enable) {
- /* enable DPU core clks */
- pm_runtime_get_sync(_kms->pdev->dev);
+ /* enable DPU core clks */
+ pm_runtime_get_sync(_kms->pdev->dev);
- /* enable all the irq */
- _dpu_encoder_irq_enable(drm_enc);
+ /* enable all the irq */
+ _dpu_encoder_irq_enable(drm_enc);
+}
- } else {
- /* disable all the irq */
- _dpu_encoder_irq_disable(drm_enc);
+static void _dpu_encoder_resource_disable(struct drm_encoder *drm_enc)
+{
+ struct msm_drm_private *priv;
+ struct dpu_kms *dpu_kms;
+ struct dpu_encoder_virt *dpu_enc;
- /* disable DPU core clks */
- pm_runtime_put_sync(_kms->pdev->dev);
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+ priv = drm_enc->dev->dev_private;
+ dpu_kms = to_dpu_kms(priv->kms);
+
+ trace_dpu_enc_rc_helper(DRMID(drm_enc), false);
and here.
+
+ if (!dpu_enc->cur_master) {
+ DPU_ERROR("encoder master not set\n");
+ return;
}
+ /* disable all the irq */
+ _dpu_encoder_irq_disable(drm_enc);
+
+ /* disable DPU core clks */
+ pm_runtime_put_sync(_kms->pdev->dev);
}
static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
@@ -847,7 +860,7 @@ static int dpu_encoder_resource_control(struct drm_encoder
*drm_enc,
if (is_vid_mode && dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE)
_dpu_encoder_irq_enable(drm_enc);
else
- _dpu_encoder_resource_control_helper(drm_enc, true);
+ _dpu_encoder_resource_enable(drm_enc);
dpu_enc->rc_state = DPU_ENC_RC_STATE_ON;
@@ -942,7 +955,7 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
* and in IDLE state the resources are already disabled
*/
if (dpu_enc->rc_state == DPU_ENC_RC_STATE_PRE_OFF)
- _dpu_encoder_resource_control_helper(drm_enc, false);
+ _dpu_encoder_resource_disable(drm_enc);
dpu_enc->rc_state = DPU_ENC_RC_STATE_OFF;
@@ -977,7 +990,7 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
if (is_vid_mode)
_dpu_encoder_irq_disable(drm_enc);
else
- _dpu_encoder_resource_control_helper(drm_enc, false);
+ _dpu_encoder_resource_disable(drm_enc);
dpu_enc->rc_state = DPU_ENC_RC_STATE_IDLE;
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: split irq_control into irq_enable and _disable
On 6/4/2023 7:45 AM, Dmitry Baryshkov wrote:
The single helper for both enable and disable cases is too complicated,
especially if we start adding more code to these helpers. Split it into
irq_enable and irq_disable cases.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 36 ---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 63 ++-
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 48 +++---
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 29 ++---
5 files changed, 112 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2e1873d29c4b..7c131c5cbe71 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -717,7 +717,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
}
}
-static void _dpu_encoder_irq_control(struct drm_encoder *drm_enc, bool enable)
+static void _dpu_encoder_irq_enable(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc;
int i;
@@ -729,14 +729,32 @@ static void _dpu_encoder_irq_control(struct drm_encoder
*drm_enc, bool enable)
dpu_enc = to_dpu_encoder_virt(drm_enc);
- DPU_DEBUG_ENC(dpu_enc, "enable:%d\n", enable);
+ DPU_DEBUG_ENC(dpu_enc, "\n");
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
- if (phys->ops.irq_control)
- phys->ops.irq_control(phys, enable);
+ phys->ops.irq_enable(phys);
+ }
+}
+
+static void _dpu_encoder_irq_disable(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc;
+ int i;
+
+ if (!drm_enc) {
+ DPU_ERROR("invalid encoder\n");
+ return;
}
+ dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ DPU_DEBUG_ENC(dpu_enc, "\n");
+ for (i = 0; i < dpu_enc->num_phys_encs; i++) {
+ struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+
+ phys->ops.irq_disable(phys);
+ }
}
static void _dpu_encoder_resource_control_helper(struct drm_encoder *drm_enc,
@@ -762,11 +780,11 @@ static void _dpu_encoder_resource_control_helper(struct
drm_encoder *drm_enc,
pm_runtime_get_sync(_kms->pdev->dev);
/* enable all the irq */
- _dpu_encoder_irq_control(drm_enc, true);
+ _dpu_encoder_irq_enable(drm_enc);
} else {
/* disable all the irq */
- _dpu_encoder_irq_control(drm_enc, false);
+ _dpu_encoder_irq_disable(drm_enc);
/* disable DPU core clks */
pm_runtime_put_sync(_kms->pdev->dev);
@@ -827,7 +845,7 @@ static int dpu_encoder_resource_control(struct drm_encoder
*drm_enc,
}
if (is_vid_mode && dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE)
- _dpu_encoder_irq_control(drm_enc, true);
+ _dpu_encoder_irq_enable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, true);
@@ -882,7 +900,7 @@ static int dpu_encoder_resource_control(struct drm_encoder *drm_enc,
if (is_vid_mode &&
dpu_enc->rc_state == DPU_ENC_RC_STATE_IDLE) {
- _dpu_encoder_irq_control(drm_enc, true);
+ _dpu_encoder_irq_enable(drm_enc);
}
/* skip if is already OFF or IDLE, resources are off already */
else if (dpu_enc->rc_state == DPU_ENC_RC_STATE_OFF ||
@@ -957,7 +975,7 @@ static int dpu_encoder_resource_control(struct drm_encoder
*drm_enc,
}
if (is_vid_mode)
- _dpu_encoder_irq_control(drm_enc, false);
+ _dpu_encoder_irq_disable(drm_enc);
else
_dpu_encoder_resource_control_helper(drm_enc, false);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index d48558ede488..faf033cd086e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -84,7 +84,8 @@ struct dpu_encoder_phys;
* @handle_post_kickoff: Do any work necessary post-kickoff work
* @trigger_start:Process start event on physical encoder
* @needs_single_flush: Whether encoder slaves need to be
flushed
- * @irq_control: Handler to enable/disable all the encoder IRQs
+ * @irq_enable:Handler to enable all the encoder IRQs
+ * @irq_disable: Handler to disable all the encoder IRQs
* @prepare_idle_pc: phys encoder can update the vsync_enable status
*
[Freedreno] [PATCH 6/7] drm/msm/dp: Inline dp_link_parse_sink_count()
The function dp_link_parse_sink_count() is really just
drm_dp_read_sink_count(). It debug prints out the bit for content
protection (DP_SINK_CP_READY), but that is not useful beyond debug
because 'link->dp_link.sink_count' is overwritten to only contain the
sink_count in this same function. Just use drm_dp_read_sink_count() in
the one place this function is called to simplify.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_link.c | 38 +++-
1 file changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 42427129acea..94a37914a47f 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -712,49 +712,17 @@ static int dp_link_parse_request(struct dp_link_private
*link)
return ret;
}
-/**
- * dp_link_parse_sink_count() - parses the sink count
- * @dp_link: pointer to link module data
- *
- * Parses the DPCD to check if there is an update to the sink count
- * (Byte 0x200), and whether all the sink devices connected have Content
- * Protection enabled.
- */
-static int dp_link_parse_sink_count(struct dp_link *dp_link)
-{
- ssize_t rlen;
- bool cp_ready;
-
- struct dp_link_private *link = container_of(dp_link,
- struct dp_link_private, dp_link);
-
- rlen = drm_dp_dpcd_readb(link->aux, DP_SINK_COUNT,
->dp_link.sink_count);
- if (rlen < 0) {
- DRM_ERROR("sink count read failed. rlen=%zd\n", rlen);
- return rlen;
- }
-
- cp_ready = link->dp_link.sink_count & DP_SINK_CP_READY;
-
- link->dp_link.sink_count =
- DP_GET_SINK_COUNT(link->dp_link.sink_count);
-
- drm_dbg_dp(link->drm_dev, "sink_count = 0x%x, cp_ready = 0x%x\n",
- link->dp_link.sink_count, cp_ready);
- return 0;
-}
-
static int dp_link_parse_sink_status_field(struct dp_link_private *link)
{
- int len = 0;
+ int len;
link->prev_sink_count = link->dp_link.sink_count;
- len = dp_link_parse_sink_count(>dp_link);
+ len = drm_dp_read_sink_count(link->aux);
if (len < 0) {
DRM_ERROR("DP parse sink count failed\n");
return len;
}
+ link->dp_link.sink_count = len;
len = drm_dp_dpcd_read_link_status(link->aux,
link->link_status);
--
https://chromeos.dev
[Freedreno] [PATCH 2/7] drm/msm/dp: Use drm_dp_read_sink_count() helper
Use the common function drm_dp_read_sink_count() instead of open-coding
it. This shrinks the kernel text a tiny bit.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_panel.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 09d4f6c38ef8..a0523b18b9e9 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -147,8 +147,8 @@ static int dp_panel_update_modes(struct drm_connector
*connector,
int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
struct drm_connector *connector)
{
- int rc = 0, bw_code;
- int rlen, count;
+ int rc, bw_code;
+ int count;
struct dp_panel_private *panel;
if (!dp_panel || !connector) {
@@ -174,16 +174,11 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
}
if (dp_panel->dfp_present) {
- rlen = drm_dp_dpcd_read(panel->aux, DP_SINK_COUNT,
- , 1);
- if (rlen == 1) {
- count = DP_GET_SINK_COUNT(count);
- if (!count) {
- DRM_ERROR("no downstream ports connected\n");
- panel->link->sink_count = 0;
- rc = -ENOTCONN;
- goto end;
- }
+ count = drm_dp_read_sink_count(panel->aux);
+ if (!count) {
+ DRM_ERROR("no downstream ports connected\n");
+ panel->link->sink_count = 0;
+ return -ENOTCONN;
}
}
--
https://chromeos.dev
[Freedreno] [PATCH 7/7] drm/msm/dp: Remove dp_display_is_ds_bridge()
This function is simply drm_dp_is_branch() so use that instead of
open-coding it.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 76f13954015b..96bbf6fec2f1 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -341,19 +341,12 @@ static const struct component_ops dp_display_comp_ops = {
.unbind = dp_display_unbind,
};
-static bool dp_display_is_ds_bridge(struct dp_panel *panel)
-{
- return (panel->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
- DP_DWN_STRM_PORT_PRESENT);
-}
-
static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
{
drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
dp->link->sink_count);
- return dp_display_is_ds_bridge(dp->panel) &&
- (dp->link->sink_count == 0);
+ return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
}
static void dp_display_send_hpd_event(struct msm_dp *dp_display)
--
https://chromeos.dev
[Freedreno] [PATCH 3/7] drm/msm/dp: Remove dead code related to downstream cap info
We read the downstream port count and capability info but never use it
anywhere. Remove 'ds_port_cnt' and 'ds_cap_info' and any associated code
from this driver. Fold the check for 'dfp_present' into a call to
drm_dp_is_branch() at the one place it is used to get rid of any member
storage related to downstream ports.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_panel.c | 25 +++--
drivers/gpu/drm/msm/dp/dp_panel.h | 6 --
2 files changed, 3 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index a0523b18b9e9..9fb4e963fefb 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -43,9 +43,7 @@ static void dp_panel_read_psr_cap(struct dp_panel_private
*panel)
static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
{
- int rc = 0;
- size_t len;
- ssize_t rlen;
+ int rc;
struct dp_panel_private *panel;
struct dp_link_info *link_info;
u8 *dpcd, major, minor;
@@ -79,25 +77,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
if (drm_dp_enhanced_frame_cap(dpcd))
link_info->capabilities |= DP_LINK_CAP_ENHANCED_FRAMING;
- dp_panel->dfp_present = dpcd[DP_DOWNSTREAMPORT_PRESENT];
- dp_panel->dfp_present &= DP_DWN_STRM_PORT_PRESENT;
-
- if (dp_panel->dfp_present && (dpcd[DP_DPCD_REV] > 0x10)) {
- dp_panel->ds_port_cnt = dpcd[DP_DOWN_STREAM_PORT_COUNT];
- dp_panel->ds_port_cnt &= DP_PORT_COUNT_MASK;
- len = DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE;
-
- rlen = drm_dp_dpcd_read(panel->aux,
- DP_DOWNSTREAM_PORT_0, dp_panel->ds_cap_info, len);
- if (rlen < len) {
- DRM_ERROR("ds port status failed, rlen=%zd\n", rlen);
- rc = -EINVAL;
- goto end;
- }
- }
-
dp_panel_read_psr_cap(panel);
-end:
+
return rc;
}
@@ -173,7 +154,7 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
return -EINVAL;
}
- if (dp_panel->dfp_present) {
+ if (drm_dp_is_branch(dp_panel->dpcd)) {
count = drm_dp_read_sink_count(panel->aux);
if (!count) {
DRM_ERROR("no downstream ports connected\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index 6d733480a62d..3cb1f8dcfd3b 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -13,9 +13,6 @@
struct edid;
-#define DP_DOWNSTREAM_PORTS4
-#define DP_DOWNSTREAM_CAP_SIZE 4
-
struct dp_display_mode {
struct drm_display_mode drm_mode;
u32 capabilities;
@@ -39,9 +36,6 @@ struct dp_panel_psr {
struct dp_panel {
/* dpcd raw data */
u8 dpcd[DP_RECEIVER_CAP_SIZE];
- u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
- u32 ds_port_cnt;
- u32 dfp_present;
struct dp_link_info link_info;
struct drm_dp_desc desc;
--
https://chromeos.dev
[Freedreno] [PATCH 4/7] drm/msm/dp: Remove aux_cfg_update_done and related code
The member 'aux_cfg_update_done' is always false. This is dead code that
never runs. Remove it.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_panel.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 9fb4e963fefb..0893522ae158 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -17,7 +17,6 @@ struct dp_panel_private {
struct dp_link *link;
struct dp_catalog *catalog;
bool panel_on;
- bool aux_cfg_update_done;
};
static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
@@ -177,19 +176,6 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
}
}
- if (panel->aux_cfg_update_done) {
- drm_dbg_dp(panel->drm_dev,
- "read DPCD with updated AUX config\n");
- rc = dp_panel_read_dpcd(dp_panel);
- bw_code = drm_dp_link_rate_to_bw_code(dp_panel->link_info.rate);
- if (rc || !is_link_rate_valid(bw_code) ||
- !is_lane_count_valid(dp_panel->link_info.num_lanes)
- || (bw_code > dp_panel->max_bw_code)) {
- DRM_ERROR("read dpcd failed %d\n", rc);
- return rc;
- }
- panel->aux_cfg_update_done = false;
- }
end:
return rc;
}
@@ -434,7 +420,6 @@ struct dp_panel *dp_panel_get(struct dp_panel_in *in)
dp_panel = >dp_panel;
dp_panel->max_bw_code = DP_LINK_BW_8_1;
- panel->aux_cfg_update_done = false;
return dp_panel;
}
--
https://chromeos.dev
[Freedreno] [PATCH 5/7] drm/msm/dp: Simplify with drm_dp_{max_link_rate, max_lane_count}()
These are open-coded versions of common functions. Replace them with the common code to improve readability. Cc: Vinod Polimera Cc: Kuogee Hsieh Signed-off-by: Stephen Boyd --- drivers/gpu/drm/msm/dp/dp_panel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c index 0893522ae158..97ba41593820 100644 --- a/drivers/gpu/drm/msm/dp/dp_panel.c +++ b/drivers/gpu/drm/msm/dp/dp_panel.c @@ -58,8 +58,8 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel) major = (link_info->revision >> 4) & 0x0f; minor = link_info->revision & 0x0f; - link_info->rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]); - link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK; + link_info->rate = drm_dp_max_link_rate(dpcd); + link_info->num_lanes = drm_dp_max_lane_count(dpcd); /* Limit data lanes from data-lanes of endpoint property of dtsi */ if (link_info->num_lanes > dp_panel->max_dp_lanes) -- https://chromeos.dev
[Freedreno] [PATCH 0/7] drm/msm/dp: Simplify DPCD related code with helpers
This driver open-codes a few of the DPCD register reads when it can be
simplified by using the helpers instead. This series reworks the MSM DP
driver to use the DPCD helpers and removes some dead code along the way.
There's the potential for even more code reduction around the test
registers, but I haven't tried to do that yet.
Stephen Boyd (7):
drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
drm/msm/dp: Use drm_dp_read_sink_count() helper
drm/msm/dp: Remove dead code related to downstream cap info
drm/msm/dp: Remove aux_cfg_update_done and related code
drm/msm/dp: Simplify with drm_dp_{max_link_rate,max_lane_count}()
drm/msm/dp: Inline dp_link_parse_sink_count()
drm/msm/dp: Remove dp_display_is_ds_bridge()
Cc: Vinod Polimera
Cc: Kuogee Hsieh
drivers/gpu/drm/msm/dp/dp_display.c | 9 +--
drivers/gpu/drm/msm/dp/dp_link.c| 38 +-
drivers/gpu/drm/msm/dp/dp_panel.c | 105 +---
drivers/gpu/drm/msm/dp/dp_panel.h | 10 +--
4 files changed, 22 insertions(+), 140 deletions(-)
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
--
https://chromeos.dev
[Freedreno] [PATCH 1/7] drm/msm/dp: Replace open-coded drm_dp_read_dpcd_caps()
This function duplicates the common function drm_dp_read_dpcd_caps().
The array of DPCD registers filled in is one size larger than the
function takes, but from what I can tell that extra byte was never used.
Resize the array and use the common function to reduce the code here.
Cc: Vinod Polimera
Cc: Kuogee Hsieh
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_panel.c | 42 ---
drivers/gpu/drm/msm/dp/dp_panel.h | 4 +--
2 files changed, 6 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 42d52510ffd4..09d4f6c38ef8 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -48,47 +48,15 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
ssize_t rlen;
struct dp_panel_private *panel;
struct dp_link_info *link_info;
- u8 *dpcd, major = 0, minor = 0, temp;
- u32 offset = DP_DPCD_REV;
+ u8 *dpcd, major, minor;
+ panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
dpcd = dp_panel->dpcd;
+ rc = drm_dp_read_dpcd_caps(panel->aux, dpcd);
+ if (rc)
+ return rc;
- panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
link_info = _panel->link_info;
-
- rlen = drm_dp_dpcd_read(panel->aux, offset,
- dpcd, (DP_RECEIVER_CAP_SIZE + 1));
- if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
- DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
- if (rlen == -ETIMEDOUT)
- rc = rlen;
- else
- rc = -EINVAL;
-
- goto end;
- }
-
- temp = dpcd[DP_TRAINING_AUX_RD_INTERVAL];
-
- /* check for EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT */
- if (temp & BIT(7)) {
- drm_dbg_dp(panel->drm_dev,
- "using EXTENDED_RECEIVER_CAPABILITY_FIELD\n");
- offset = DPRX_EXTENDED_DPCD_FIELD;
- }
-
- rlen = drm_dp_dpcd_read(panel->aux, offset,
- dpcd, (DP_RECEIVER_CAP_SIZE + 1));
- if (rlen < (DP_RECEIVER_CAP_SIZE + 1)) {
- DRM_ERROR("dpcd read failed, rlen=%zd\n", rlen);
- if (rlen == -ETIMEDOUT)
- rc = rlen;
- else
- rc = -EINVAL;
-
- goto end;
- }
-
link_info->revision = dpcd[DP_DPCD_REV];
major = (link_info->revision >> 4) & 0x0f;
minor = link_info->revision & 0x0f;
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
b/drivers/gpu/drm/msm/dp/dp_panel.h
index ed1030e17e1b..6d733480a62d 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -13,8 +13,6 @@
struct edid;
-#define DPRX_EXTENDED_DPCD_FIELD 0x2200
-
#define DP_DOWNSTREAM_PORTS4
#define DP_DOWNSTREAM_CAP_SIZE 4
@@ -40,7 +38,7 @@ struct dp_panel_psr {
struct dp_panel {
/* dpcd raw data */
- u8 dpcd[DP_RECEIVER_CAP_SIZE + 1];
+ u8 dpcd[DP_RECEIVER_CAP_SIZE];
u8 ds_cap_info[DP_DOWNSTREAM_PORTS * DP_DOWNSTREAM_CAP_SIZE];
u32 ds_port_cnt;
u32 dfp_present;
--
https://chromeos.dev
Re: [Freedreno] [PATCH RFC v6 00/10] Support for Solid Fill Planes
On Mon, Aug 28, 2023 at 05:05:06PM -0700, Jessica Zhang wrote: > Some drivers support hardware that have optimizations for solid fill > planes. This series aims to expose these capabilities to userspace as > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > hardware composer HAL) that can be set by apps like the Android Gears > app. > > In order to expose this capability to userspace, this series will: > > - Introduce solid_fill and pixel_source properties to allow userspace to > toggle between FB and solid fill sources > - Loosen NULL FB checks within the DRM atomic commit callstack to allow > for NULL FB when solid fill is enabled. > - Add NULL FB checks in methods where FB was previously assumed to be > non-NULL > - Have MSM DPU driver use drm_plane_state.solid_fill instead of > dpu_plane_state.color_fill > > Note: The solid fill planes feature depends on both the solid_fill *and* > pixel_source properties. > > To use this feature, userspace can set the solid_fill property to a blob > containing the appropriate version number and solid fill color (in > RGB323232 format) and and setting the pixel_source property to > DRM_PLANE_PIXEL_SOURCE_COLOR. This will disable memory fetch and the > resulting plane will display the color specified by the solid_fill blob. > > Currently, there's only one version of the solid_fill blob property. > However if other drivers want to support a similar feature, but require > more than just the solid fill color, they can extend this feature by > creating additional versions of the drm_solid_fill struct. > > This 2 property approach was chosen because passing in a special 1x1 FB > with the necessary color information would have unecessary overhead that > does not reflect the behavior of the solid fill feature. In addition, > assigning the solid fill blob to FB_ID would require loosening some core > drm_property checks that might cause unwanted side effects elsewhere. The cover letter is a bit outdated by now. Anyway, with Pekkas issues addressed the core drm parts are Acked-by: Sebastian Wick > --- > Changes in v6: > - Have _dpu_plane_color_fill() take in a single ABGR color instead > of having separate alpha and BGR color parameters (Dmitry) > - Drop plane->state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB check > in SetPlane ioctl (Dmitry) > - Add DRM_PLANE_PIXEL_SOURCE_NONE as a default pixel source (Sebastian) > - Dropped versioning from solid fill property blob (Dmitry) > - Use DRM_ENUM_NAME_FN (Dmitry) > - Use drm_atomic_replace_property_blob_from_id() (Dmitry) > - drm_atomic_check_fb -> drm_atomic_plane_check_fb (Dmitry) > - Group redundant NULL FB checks (Dmitry) > - Squashed drm_plane_needs_disable() implementation with > DRM_PLANE_PIXEL_SOURCE_NONE declaration (Sebastian) > - Add comment to support RGBA solid fill color in the future (Dmitry) > - Link to v5: > https://lore.kernel.org/r/20230728-solid-fill-v5-0-053dbefa9...@quicinc.com > > Changes in v5: > - Added support for PIXEL_SOURCE_NONE (Sebastian) > - Added WARN_ON() in drm_plane_has_visible_data() if pixel_source isn't > set (Dmitry) > - Added debugfs support for both properties (Dmitry) > - Corrected u32 to u8 conversion (Pekka) > - Moved drm_solid_fill_info struct and related documentation to > include/uapi (Pekka) > - Changed drm_solid_fill_info.version to __u32 for data alignment (Pekka) > - Added more detailed UAPI and kernel documentation (Pekka) > - Reordered patch series so that the pixel_source property is introduced > before solid_fill (Dmitry) > - Fixed inconsistent ABGR/RGBA format declaration (Pekka) > - Reset pixel_source to FB in drm_mode_setplane() (Dmitry) > - Rename supported_sources to extra_sources (Dmitry) > - Only destroy old solid_fill blob state if new state is valid (Pekka) > - Link to v4: > https://lore.kernel.org/r/20230404-solid-fill-v4-0-f4ec0caa7...@quicinc.com > > Changes in v4: > - Rebased onto latest kernel > - Reworded cover letter for clarity (Dmitry) > - Reworded commit messages for clarity > - Split existing changes into smaller commits > - Added pixel_source enum property (Dmitry, Pekka, Ville) > - Updated drm-kms comment docs with pixel_source and solid_fill > properties (Dmitry) > - Inlined drm_atomic_convert_solid_fill_info() (Dmitry) > - Passed in plane state alpha value to _dpu_plane_color_fill_pipe() > - Link to v3: > https://lore.kernel.org/r/20230104234036.636-1-quic_jessz...@quicinc.com > > Changes in v3: > - Fixed some logic errors in atomic checks (Dmitry) > - Introduced drm_plane_has_visible_data() and drm_atomic_check_fb() helper > methods (Dmitry) > - Fixed typo in drm_solid_fill struct documentation > - Created drm_plane_has_visible_data() helper and corrected CRTC and FB > NULL-check logic (Dmitry) > - Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted > them into helper method (Dmitry) > - Inverted `if (solid_fill_enabled) else if (fb)` check order (Dmitry) >
Re: [Freedreno] [PATCH RFC v6 02/10] drm: Introduce solid fill DRM plane property
On Mon, Aug 28, 2023 at 05:05:08PM -0700, Jessica Zhang wrote:
> Document and add support for solid_fill property to drm_plane. In
> addition, add support for setting and getting the values for solid_fill.
>
> To enable solid fill planes, userspace must assign a property blob to
> the "solid_fill" plane property containing the following information:
>
> struct drm_mode_solid_fill {
> u32 r, g, b;
> };
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 9
> drivers/gpu/drm/drm_atomic_uapi.c | 26 ++
> drivers/gpu/drm/drm_blend.c | 30 ++
> include/drm/drm_blend.h | 1 +
> include/drm/drm_plane.h | 36
> +++
> include/uapi/drm/drm_mode.h | 24 +
> 6 files changed, 126 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 01638c51ce0a..86fb876efbe6 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -254,6 +254,11 @@ void __drm_atomic_helper_plane_state_reset(struct
> drm_plane_state *plane_state,
> plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
>
> + if (plane_state->solid_fill_blob) {
> + drm_property_blob_put(plane_state->solid_fill_blob);
> + plane_state->solid_fill_blob = NULL;
> + }
> +
> if (plane->color_encoding_property) {
> if (!drm_object_property_get_default_value(>base,
>
> plane->color_encoding_property,
> @@ -336,6 +341,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> if (state->fb)
> drm_framebuffer_get(state->fb);
>
> + if (state->solid_fill_blob)
> + drm_property_blob_get(state->solid_fill_blob);
> +
> state->fence = NULL;
> state->commit = NULL;
> state->fb_damage_clips = NULL;
> @@ -385,6 +393,7 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
> drm_crtc_commit_put(state->commit);
>
> drm_property_blob_put(state->fb_damage_clips);
> + drm_property_blob_put(state->solid_fill_blob);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 454f980e16c9..1cae596ab693 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -316,6 +316,20 @@ drm_atomic_set_crtc_for_connector(struct
> drm_connector_state *conn_state,
> }
> EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
>
> +static void drm_atomic_set_solid_fill_prop(struct drm_plane_state *state)
> +{
> + struct drm_mode_solid_fill *user_info;
> +
> + if (!state->solid_fill_blob)
> + return;
> +
> + user_info = (struct drm_mode_solid_fill *)state->solid_fill_blob->data;
> +
> + state->solid_fill.r = user_info->r;
> + state->solid_fill.g = user_info->g;
> + state->solid_fill.b = user_info->b;
> +}
> +
> static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> struct drm_crtc *crtc, s32 __user *fence_ptr)
> {
> @@ -546,6 +560,15 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> state->src_h = val;
> } else if (property == plane->pixel_source_property) {
> state->pixel_source = val;
> + } else if (property == plane->solid_fill_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + >solid_fill_blob,
> + val, sizeof(struct drm_mode_solid_fill),
> + -1, );
> + if (ret)
> + return ret;
> +
> + drm_atomic_set_solid_fill_prop(state);
> } else if (property == plane->alpha_property) {
> state->alpha = val;
> } else if (property == plane->blend_mode_property) {
> @@ -620,6 +643,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->src_h;
> } else if (property == plane->pixel_source_property) {
> *val = state->pixel_source;
> + } else if (property == plane->solid_fill_property) {
> + *val = state->solid_fill_blob ?
> + state->solid_fill_blob->base.id : 0;
> } else if (property == plane->alpha_property) {
> *val = state->alpha;
> } else if (property == plane->blend_mode_property) {
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index c3c57bae06b7..273021cc21c8 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -200,6 +200,10 @@
Re: [Freedreno] [PATCH RFC v6 07/10] drm/atomic: Loosen FB atomic checks
On Mon, 28 Aug 2023 17:05:13 -0700
Jessica Zhang wrote:
> Loosen the requirements for atomic and legacy commit so that, in cases
> where pixel_source != FB, the commit can still go through.
>
> This includes adding framebuffer NULL checks in other areas to account for
> FB being NULL when non-FB pixel sources are enabled.
>
> To disable a plane, the pixel_source must be NONE or the FB must be NULL
> if pixel_source == FB.
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/drm_atomic.c| 20 +++-
> drivers/gpu/drm/drm_atomic_helper.c | 36
> include/drm/drm_atomic_helper.h | 4 ++--
> include/drm/drm_plane.h | 29 +
> 4 files changed, 62 insertions(+), 27 deletions(-)
...
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index a58f84b6bd5e..4c5b7bcdb25c 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -992,6 +992,35 @@ static inline struct drm_plane *drm_plane_find(struct
> drm_device *dev,
> #define drm_for_each_plane(plane, dev) \
> list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
>
> +/**
> + * drm_plane_solid_fill_enabled - Check if solid fill is enabled on plane
> + * @state: plane state
> + *
> + * Returns:
> + * Whether the plane has been assigned a solid_fill_blob
> + */
> +static inline bool drm_plane_solid_fill_enabled(struct drm_plane_state
> *state)
> +{
> + if (!state)
> + return false;
> + return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL &&
> state->solid_fill_blob;
> +}
> +
> +static inline bool drm_plane_has_visible_data(const struct drm_plane_state
> *state)
> +{
> + switch (state->pixel_source) {
> + case DRM_PLANE_PIXEL_SOURCE_NONE:
> + return false;
> + case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
> + return state->solid_fill_blob != NULL;
This reminds me, new UAPI docs did not say what the requirements are for
choosing solid fill pixel source. Is the atomic commit rejected if
pixel source is solid fill, but solid_fill property has no blob?
This should be doc'd.
Thanks,
pq
> + case DRM_PLANE_PIXEL_SOURCE_FB:
> + default:
> + WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
> + }
> +
> + return state->fb != NULL;
> +}
> +
> bool drm_any_plane_has_format(struct drm_device *dev,
> u32 format, u64 modifier);
>
>
pgpnBQJa4lnTk.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH RFC v6 05/10] drm/atomic: Add solid fill data to plane state dump
On Mon, 28 Aug 2023 17:05:11 -0700
Jessica Zhang wrote:
> Add solid_fill property data to the atomic plane state dump.
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/drm_atomic.c | 4
> drivers/gpu/drm/drm_plane.c | 8
> include/drm/drm_plane.h | 3 +++
> 3 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index bcecb64ccfad..3cb599b3304a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -717,6 +717,10 @@ static void drm_atomic_plane_print_state(struct
> drm_printer *p,
> drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0);
> if (state->fb)
> drm_framebuffer_print_info(p, 2, state->fb);
> + drm_printf(p, "\tsolid_fill=%u\n",
> + state->solid_fill_blob ?
> state->solid_fill_blob->base.id : 0);
> + if (state->solid_fill_blob)
> + drm_plane_solid_fill_print_info(p, 2, state);
> drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG());
> drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG());
> drm_printf(p, "\trotation=%x\n", state->rotation);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 559d101162ba..6244b622a21a 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1495,6 +1495,14 @@ __drm_plane_get_damage_clips(const struct
> drm_plane_state *state)
> state->fb_damage_clips->data : NULL);
> }
>
> +void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int
> indent,
> + const struct drm_plane_state *state)
> +{
> + drm_printf_indent(p, indent, "r=0x%x\n", state->solid_fill.r);
> + drm_printf_indent(p, indent, "g=0x%x\n", state->solid_fill.g);
> + drm_printf_indent(p, indent, "b=0x%x\n", state->solid_fill.b);
I'd recommend %08x format, so that leading zeroes are explicit. That
makes it easier to see the difference between e.g. 0x and
0x0fff.
Thanks,
pq
> +}
> +
> /**
> * drm_plane_get_damage_clips - Returns damage clips.
> * @state: Plane state.
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 49995c4be2ab..a58f84b6bd5e 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -1001,6 +1001,9 @@ drm_plane_get_damage_clips_count(const struct
> drm_plane_state *state);
> struct drm_mode_rect *
> drm_plane_get_damage_clips(const struct drm_plane_state *state);
>
> +void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int
> indent,
> + const struct drm_plane_state *state);
> +
> int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
>unsigned int supported_filters);
>
>
pgpCVL2IUjmzq.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH RFC v6 03/10] drm: Add solid fill pixel source
On Mon, 28 Aug 2023 17:05:09 -0700
Jessica Zhang wrote:
> Add "SOLID_FILL" as a valid pixel source. If the pixel_source property is
> set to "SOLID_FILL", it will display data from the drm_plane "solid_fill"
> blob property.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/drm_blend.c | 10 +-
> include/drm/drm_plane.h | 1 +
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 273021cc21c8..1016a206ca0c 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -200,6 +200,9 @@
> * "FB":
> * Framebuffer source set by the "FB_ID" property.
> *
> + * "SOLID_FILL":
> + * Solid fill color source set by the "solid_fill" property.
> + *
> * solid_fill:
> * solid_fill is set up with drm_plane_create_solid_fill_property(). It
> * contains pixel data that drivers can use to fill a plane.
> @@ -638,6 +641,7 @@ EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
> static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
> { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
> { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
> + { DRM_PLANE_PIXEL_SOURCE_SOLID_FILL, "SOLID_FILL" },
> };
>
> /**
> @@ -662,6 +666,9 @@ static const struct drm_prop_enum_list
> drm_pixel_source_enum_list[] = {
> * "FB":
> * Framebuffer pixel source
> *
> + * "SOLID_FILL":
> + * Solid fill color pixel source
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> @@ -671,7 +678,8 @@ int drm_plane_create_pixel_source_property(struct
> drm_plane *plane,
> struct drm_device *dev = plane->dev;
> struct drm_property *prop;
> static const unsigned int valid_source_mask =
> BIT(DRM_PLANE_PIXEL_SOURCE_FB) |
> -
> BIT(DRM_PLANE_PIXEL_SOURCE_NONE);
> +
> BIT(DRM_PLANE_PIXEL_SOURCE_NONE) |
> +
> BIT(DRM_PLANE_PIXEL_SOURCE_SOLID_FILL);
> int i;
>
> /* FB is supported by default */
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index a38e18bfb43e..49995c4be2ab 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -43,6 +43,7 @@ enum drm_scaling_filter {
> enum drm_plane_pixel_source {
> DRM_PLANE_PIXEL_SOURCE_NONE,
> DRM_PLANE_PIXEL_SOURCE_FB,
> + DRM_PLANE_PIXEL_SOURCE_SOLID_FILL,
> DRM_PLANE_PIXEL_SOURCE_MAX
> };
>
>
This UAPI:
Acked-by: Pekka Paalanen
Thanks,
pq
pgpMr0DOxFDEg.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH RFC v6 02/10] drm: Introduce solid fill DRM plane property
On Mon, 28 Aug 2023 17:05:08 -0700
Jessica Zhang wrote:
> Document and add support for solid_fill property to drm_plane. In
> addition, add support for setting and getting the values for solid_fill.
>
> To enable solid fill planes, userspace must assign a property blob to
> the "solid_fill" plane property containing the following information:
>
> struct drm_mode_solid_fill {
> u32 r, g, b;
> };
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 9
> drivers/gpu/drm/drm_atomic_uapi.c | 26 ++
> drivers/gpu/drm/drm_blend.c | 30 ++
> include/drm/drm_blend.h | 1 +
> include/drm/drm_plane.h | 36
> +++
> include/uapi/drm/drm_mode.h | 24 +
> 6 files changed, 126 insertions(+)
...
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index c3c57bae06b7..273021cc21c8 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -200,6 +200,10 @@
> * "FB":
> * Framebuffer source set by the "FB_ID" property.
> *
> + * solid_fill:
> + * solid_fill is set up with drm_plane_create_solid_fill_property(). It
> + * contains pixel data that drivers can use to fill a plane.
> + *
> * Note that all the property extensions described here apply either to the
> * plane or the CRTC (e.g. for the background color, which currently is not
> * exposed and assumed to be black).
...
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 43691058d28f..1fd92886d66c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -259,6 +259,30 @@ struct drm_mode_modeinfo {
> char name[DRM_DISPLAY_MODE_LEN];
> };
>
> +/**
> + * struct drm_mode_solid_fill - User info for solid fill planes
> + *
> + * This is the userspace API solid fill information structure.
> + *
> + * Userspace can enable solid fill planes by assigning the plane "solid_fill"
> + * property to a blob containing a single drm_mode_solid_fill struct
> populated with an RGB323232
> + * color and setting the pixel source to "SOLID_FILL".
> + *
> + * For information on the plane property, see
> drm_plane_create_solid_fill_property()
> + *
> + * @r: Red color value of single pixel
> + * @g: Green color value of single pixel
> + * @b: Blue color value of single pixel
> + * @pad: padding
Document that padding must be zero, and ensure userspace obeys that. If
there is ever need to re-purpose the pad field, requiring it to be zero
today makes re-purposing possible.
Alternatively, if it is likely that it might be used as alpha if
alpha-ful format is added, then it would make more sense to require it
to be 0x instead of zero. Then the kernel could internally
interpret it as alpha always without special-casing zero. Or, it could
be straight out called "alpha" to begin with, but document and verify
that it must be set to 0x which means opaque.
> + */
> +struct drm_mode_solid_fill {
> + __u32 r;
> + __u32 g;
> + __u32 b;
> + __u32 pad;
> +};
> +
> +
> struct drm_mode_card_res {
> __u64 fb_id_ptr;
> __u64 crtc_id_ptr;
>
Looking good.
Thanks,
pq
pgpPzTOhaxwNV.pgp
Description: OpenPGP digital signature
Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property
On Mon, 28 Aug 2023 17:05:07 -0700 Jessica Zhang wrote: > Add support for pixel_source property to drm_plane and related > documentation. In addition, force pixel_source to > DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break > legacy userspace. > > This enum property will allow user to specify a pixel source for the > plane. Possible pixel sources will be defined in the > drm_plane_pixel_source enum. > > Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the > default value) and DRM_PLANE_PIXEL_SOURCE_NONE. > > Signed-off-by: Jessica Zhang > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > drivers/gpu/drm/drm_blend.c | 90 > +++ > drivers/gpu/drm/drm_plane.c | 19 +-- > include/drm/drm_blend.h | 2 + > include/drm/drm_plane.h | 21 > 6 files changed, 133 insertions(+), 4 deletions(-) ... > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index 6e74de833466..c3c57bae06b7 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -185,6 +185,21 @@ > *plane does not expose the "alpha" property, then this is > *assumed to be 1.0 > * > + * pixel_source: > + * pixel_source is set up with drm_plane_create_pixel_source_property(). > + * It is used to toggle the active source of pixel data for the plane. > + * The plane will only display data from the set pixel_source -- any > + * data from other sources will be ignored. > + * > + * Possible values: > + * > + * "NONE": > + * No active pixel source. > + * Committing with a NONE pixel source will disable the plane. > + * > + * "FB": > + * Framebuffer source set by the "FB_ID" property. > + * > * Note that all the property extensions described here apply either to the > * plane or the CRTC (e.g. for the background color, which currently is not > * exposed and assumed to be black). This UAPI: Acked-by: Pekka Paalanen Thanks, pq pgpIXA2uzDNp7.pgp Description: OpenPGP digital signature
