Re: [Freedreno] [PATCH 14/25] drm/msm/dpu: remove enc_id tagging for hw blocks
On 2018-10-10 08:06, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:31PM -0700, Jeykumar Sankaran wrote:
RM was using encoder id's to tag HW block's to reserve
and retrieve later for display pipeline. Now
that all the reserved HW blocks for a display are
maintained in its crtc state, no retrieval is needed.
This patch cleans up RM of encoder id tagging.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 90
+--
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 --
2 files changed, 36 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 303f1b3..a8461b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -21,9 +21,6 @@
#include "dpu_encoder.h"
#include "dpu_trace.h"
-#define RESERVED_BY_OTHER(h, r) \
- ((h)->enc_id && (h)->enc_id != r)
-
/**
* struct dpu_rm_requirements - Reservation requirements parameter
bundle
* @topology: selected topology for the display
@@ -38,12 +35,13 @@ struct dpu_rm_requirements {
/**
* struct dpu_rm_hw_blk - hardware block tracking list member
* @list: List head for list of all hardware blocks tracking items
- * @enc_id:Encoder id to which this blk is binded
+ * @in_use: True, if the hw block is assigned to a display
pipeline.
+ * False, otherwise
* @hw:Pointer to the hardware register access object for
this block
*/
struct dpu_rm_hw_blk {
struct list_head list;
- uint32_t enc_id;
+ bool in_use;
How do the reservations work for TEST_ONLY commits? At a quick glance
it
looks
like they might be marked in_use?
Yes. We have a bug. I guess I should be releasing them in
drm_crtc_destroy_state.
Thanks and Regards,
Jeykumar S.
Sean
struct dpu_hw_blk *hw;
};
@@ -51,23 +49,19 @@ struct dpu_rm_hw_blk {
* struct dpu_rm_hw_iter - iterator for use with dpu_rm
* @hw: dpu_hw object requested, or NULL on failure
* @blk: dpu_rm internal block representation. Clients ignore. Used
as
iterator.
- * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for
Any
Encoder
* @type: Hardware Block Type client wishes to search for.
*/
struct dpu_rm_hw_iter {
struct dpu_hw_blk *hw;
struct dpu_rm_hw_blk *blk;
- uint32_t enc_id;
enum dpu_hw_blk_type type;
};
static void _dpu_rm_init_hw_iter(
struct dpu_rm_hw_iter *iter,
- uint32_t enc_id,
enum dpu_hw_blk_type type)
{
memset(iter, 0, sizeof(*iter));
- iter->enc_id = enc_id;
iter->type = type;
}
@@ -91,16 +85,12 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm
*rm,
struct dpu_rm_hw_iter *i)
i->blk = list_prepare_entry(i->blk, blk_list, list);
list_for_each_entry_continue(i->blk, blk_list, list) {
- if (i->enc_id == i->blk->enc_id) {
+ if (!i->blk->in_use) {
i->hw = i->blk->hw;
- DPU_DEBUG("found type %d id %d for enc %d\n",
- i->type, i->blk->hw->id,
i->enc_id);
return true;
}
}
- DPU_DEBUG("no match, type %d for enc %d\n", i->type, i->enc_id);
-
return false;
}
@@ -196,7 +186,6 @@ static int _dpu_rm_hw_blk_create(
}
blk->hw = hw;
- blk->enc_id = 0;
list_add_tail(>list, >hw_blks[type]);
return 0;
@@ -301,7 +290,6 @@ static bool _dpu_rm_needs_split_display(const
struct
msm_display_topology *top)
* proposed use case requirements, incl. hardwired dependent blocks
like
* pingpong
* @rm: dpu resource manager handle
- * @enc_id: encoder id requesting for allocation
* @reqs: proposed use case requirements
* @lm: proposed layer mixer, function checks if lm, and all other
hardwired
* blocks connected to the lm (pp) is available and appropriate
@@ -313,7 +301,6 @@ static bool _dpu_rm_needs_split_display(const
struct
msm_display_topology *top)
*/
static bool _dpu_rm_check_lm_and_get_connected_blks(
struct dpu_rm *rm,
- uint32_t enc_id,
struct dpu_rm_requirements *reqs,
struct dpu_rm_hw_blk *lm,
struct dpu_rm_hw_blk **pp,
@@ -339,13 +326,7 @@ static bool
_dpu_rm_check_lm_and_get_connected_blks(
}
}
- /* Already reserved? */
- if (RESERVED_BY_OTHER(lm, enc_id)) {
- DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
- return false;
- }
-
- _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
+ _dpu_rm_init_hw_iter(, DPU_HW_BLK_PINGPONG);
while (_dpu_rm_get_hw_locked(rm, )) {
if (iter.blk->hw->id == lm_cfg->pingpong) {
*pp = iter.blk;
@@ -358,16 +339,10 @@
Re: [Freedreno] [PATCH 14/25] drm/msm/dpu: remove enc_id tagging for hw blocks
On Mon, Oct 08, 2018 at 09:27:31PM -0700, Jeykumar Sankaran wrote:
> RM was using encoder id's to tag HW block's to reserve
> and retrieve later for display pipeline. Now
> that all the reserved HW blocks for a display are
> maintained in its crtc state, no retrieval is needed.
> This patch cleans up RM of encoder id tagging.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 90
> +--
> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 --
> 2 files changed, 36 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 303f1b3..a8461b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -21,9 +21,6 @@
> #include "dpu_encoder.h"
> #include "dpu_trace.h"
>
> -#define RESERVED_BY_OTHER(h, r) \
> - ((h)->enc_id && (h)->enc_id != r)
> -
> /**
> * struct dpu_rm_requirements - Reservation requirements parameter bundle
> * @topology: selected topology for the display
> @@ -38,12 +35,13 @@ struct dpu_rm_requirements {
> /**
> * struct dpu_rm_hw_blk - hardware block tracking list member
> * @list:List head for list of all hardware blocks tracking items
> - * @enc_id: Encoder id to which this blk is binded
> + * @in_use: True, if the hw block is assigned to a display pipeline.
> + * False, otherwise
> * @hw: Pointer to the hardware register access object for this
> block
> */
> struct dpu_rm_hw_blk {
> struct list_head list;
> - uint32_t enc_id;
> + bool in_use;
How do the reservations work for TEST_ONLY commits? At a quick glance it looks
like they might be marked in_use?
Sean
> struct dpu_hw_blk *hw;
> };
>
> @@ -51,23 +49,19 @@ struct dpu_rm_hw_blk {
> * struct dpu_rm_hw_iter - iterator for use with dpu_rm
> * @hw: dpu_hw object requested, or NULL on failure
> * @blk: dpu_rm internal block representation. Clients ignore. Used as
> iterator.
> - * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for Any
> Encoder
> * @type: Hardware Block Type client wishes to search for.
> */
> struct dpu_rm_hw_iter {
> struct dpu_hw_blk *hw;
> struct dpu_rm_hw_blk *blk;
> - uint32_t enc_id;
> enum dpu_hw_blk_type type;
> };
>
> static void _dpu_rm_init_hw_iter(
> struct dpu_rm_hw_iter *iter,
> - uint32_t enc_id,
> enum dpu_hw_blk_type type)
> {
> memset(iter, 0, sizeof(*iter));
> - iter->enc_id = enc_id;
> iter->type = type;
> }
>
> @@ -91,16 +85,12 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm,
> struct dpu_rm_hw_iter *i)
> i->blk = list_prepare_entry(i->blk, blk_list, list);
>
> list_for_each_entry_continue(i->blk, blk_list, list) {
> - if (i->enc_id == i->blk->enc_id) {
> + if (!i->blk->in_use) {
> i->hw = i->blk->hw;
> - DPU_DEBUG("found type %d id %d for enc %d\n",
> - i->type, i->blk->hw->id, i->enc_id);
> return true;
> }
> }
>
> - DPU_DEBUG("no match, type %d for enc %d\n", i->type, i->enc_id);
> -
> return false;
> }
>
> @@ -196,7 +186,6 @@ static int _dpu_rm_hw_blk_create(
> }
>
> blk->hw = hw;
> - blk->enc_id = 0;
> list_add_tail(>list, >hw_blks[type]);
>
> return 0;
> @@ -301,7 +290,6 @@ static bool _dpu_rm_needs_split_display(const struct
> msm_display_topology *top)
> * proposed use case requirements, incl. hardwired dependent blocks like
> * pingpong
> * @rm: dpu resource manager handle
> - * @enc_id: encoder id requesting for allocation
> * @reqs: proposed use case requirements
> * @lm: proposed layer mixer, function checks if lm, and all other hardwired
> * blocks connected to the lm (pp) is available and appropriate
> @@ -313,7 +301,6 @@ static bool _dpu_rm_needs_split_display(const struct
> msm_display_topology *top)
> */
> static bool _dpu_rm_check_lm_and_get_connected_blks(
> struct dpu_rm *rm,
> - uint32_t enc_id,
> struct dpu_rm_requirements *reqs,
> struct dpu_rm_hw_blk *lm,
> struct dpu_rm_hw_blk **pp,
> @@ -339,13 +326,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
> }
> }
>
> - /* Already reserved? */
> - if (RESERVED_BY_OTHER(lm, enc_id)) {
> - DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
> - return false;
> - }
> -
> - _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
> + _dpu_rm_init_hw_iter(, DPU_HW_BLK_PINGPONG);
> while (_dpu_rm_get_hw_locked(rm, )) {
> if (iter.blk->hw->id == lm_cfg->pingpong) {
> *pp = iter.blk;
> @@ -358,16 +339,10 @@ static bool
[Freedreno] [PATCH 14/25] drm/msm/dpu: remove enc_id tagging for hw blocks
RM was using encoder id's to tag HW block's to reserve
and retrieve later for display pipeline. Now
that all the reserved HW blocks for a display are
maintained in its crtc state, no retrieval is needed.
This patch cleans up RM of encoder id tagging.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 90 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 --
2 files changed, 36 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 303f1b3..a8461b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -21,9 +21,6 @@
#include "dpu_encoder.h"
#include "dpu_trace.h"
-#define RESERVED_BY_OTHER(h, r) \
- ((h)->enc_id && (h)->enc_id != r)
-
/**
* struct dpu_rm_requirements - Reservation requirements parameter bundle
* @topology: selected topology for the display
@@ -38,12 +35,13 @@ struct dpu_rm_requirements {
/**
* struct dpu_rm_hw_blk - hardware block tracking list member
* @list: List head for list of all hardware blocks tracking items
- * @enc_id:Encoder id to which this blk is binded
+ * @in_use: True, if the hw block is assigned to a display pipeline.
+ * False, otherwise
* @hw:Pointer to the hardware register access object for this
block
*/
struct dpu_rm_hw_blk {
struct list_head list;
- uint32_t enc_id;
+ bool in_use;
struct dpu_hw_blk *hw;
};
@@ -51,23 +49,19 @@ struct dpu_rm_hw_blk {
* struct dpu_rm_hw_iter - iterator for use with dpu_rm
* @hw: dpu_hw object requested, or NULL on failure
* @blk: dpu_rm internal block representation. Clients ignore. Used as
iterator.
- * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for Any Encoder
* @type: Hardware Block Type client wishes to search for.
*/
struct dpu_rm_hw_iter {
struct dpu_hw_blk *hw;
struct dpu_rm_hw_blk *blk;
- uint32_t enc_id;
enum dpu_hw_blk_type type;
};
static void _dpu_rm_init_hw_iter(
struct dpu_rm_hw_iter *iter,
- uint32_t enc_id,
enum dpu_hw_blk_type type)
{
memset(iter, 0, sizeof(*iter));
- iter->enc_id = enc_id;
iter->type = type;
}
@@ -91,16 +85,12 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm, struct
dpu_rm_hw_iter *i)
i->blk = list_prepare_entry(i->blk, blk_list, list);
list_for_each_entry_continue(i->blk, blk_list, list) {
- if (i->enc_id == i->blk->enc_id) {
+ if (!i->blk->in_use) {
i->hw = i->blk->hw;
- DPU_DEBUG("found type %d id %d for enc %d\n",
- i->type, i->blk->hw->id, i->enc_id);
return true;
}
}
- DPU_DEBUG("no match, type %d for enc %d\n", i->type, i->enc_id);
-
return false;
}
@@ -196,7 +186,6 @@ static int _dpu_rm_hw_blk_create(
}
blk->hw = hw;
- blk->enc_id = 0;
list_add_tail(>list, >hw_blks[type]);
return 0;
@@ -301,7 +290,6 @@ static bool _dpu_rm_needs_split_display(const struct
msm_display_topology *top)
* proposed use case requirements, incl. hardwired dependent blocks like
* pingpong
* @rm: dpu resource manager handle
- * @enc_id: encoder id requesting for allocation
* @reqs: proposed use case requirements
* @lm: proposed layer mixer, function checks if lm, and all other hardwired
* blocks connected to the lm (pp) is available and appropriate
@@ -313,7 +301,6 @@ static bool _dpu_rm_needs_split_display(const struct
msm_display_topology *top)
*/
static bool _dpu_rm_check_lm_and_get_connected_blks(
struct dpu_rm *rm,
- uint32_t enc_id,
struct dpu_rm_requirements *reqs,
struct dpu_rm_hw_blk *lm,
struct dpu_rm_hw_blk **pp,
@@ -339,13 +326,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
}
}
- /* Already reserved? */
- if (RESERVED_BY_OTHER(lm, enc_id)) {
- DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
- return false;
- }
-
- _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
+ _dpu_rm_init_hw_iter(, DPU_HW_BLK_PINGPONG);
while (_dpu_rm_get_hw_locked(rm, )) {
if (iter.blk->hw->id == lm_cfg->pingpong) {
*pp = iter.blk;
@@ -358,16 +339,10 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
return false;
}
- if (RESERVED_BY_OTHER(*pp, enc_id)) {
- DPU_DEBUG("lm %d pp %d already reserved\n", lm->hw->id,
- (*pp)->hw->id);
- return false;
- }
-
return true;
}
-static int _dpu_rm_reserve_lms(struct
