Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Support DisplayPort (non-eDP) mode
Quoting Kieran Bingham (2022-03-07 17:59:54) > From: Laurent Pinchart > > Despite the SN65DSI86 being an eDP bridge, on some systems its output is > routed to a DisplayPort connector. Enable DisplayPort mode when the next > component in the display pipeline is detected as a DisplayPort > connector, and disable eDP features in that case. > > Signed-off-by: Laurent Pinchart > Reworked to set bridge type based on the next bridge/connector. > Signed-off-by: Kieran Bingham > -- > Changes since v1/RFC: > - Rebased on top of "drm/bridge: ti-sn65dsi86: switch to >devm_drm_of_get_bridge" > - eDP/DP mode determined from the next bridge connector type. > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 29f5f7123ed9..461f963faa0b 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -60,6 +60,7 @@ > #define SN_LN_ASSIGN_REG 0x59 > #define LN_ASSIGN_WIDTH 2 > #define SN_ENH_FRAME_REG 0x5A > +#define ASSR_CONTROL BIT(0) > #define VSTREAM_ENABLEBIT(3) > #define LN_POLRS_OFFSET 4 > #define LN_POLRS_MASK 0xf0 > @@ -91,6 +92,8 @@ > #define SN_DATARATE_CONFIG_REG 0x94 > #define DP_DATARATE_MASK GENMASK(7, 5) > #define DP_DATARATE(x)((x) << 5) > +#define SN_TRAINING_SETTING_REG0x95 > +#define SCRAMBLE_DISABLE BIT(4) > #define SN_ML_TX_MODE_REG 0x96 > #define ML_TX_MAIN_LINK_OFF 0 > #define ML_TX_NORMAL_MODE BIT(0) > @@ -1005,6 +1008,11 @@ static int ti_sn_link_training(struct ti_sn65dsi86 > *pdata, int dp_rate_idx, > regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG, >DP_DATARATE_MASK, DP_DATARATE(dp_rate_idx)); > > + /* For DisplayPort, use the standard DP scrambler seed. */ > + if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) > + regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, > + ASSR_CONTROL, 0); > + > /* enable DP PLL */ > regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1); > > @@ -1016,6 +1024,11 @@ static int ti_sn_link_training(struct ti_sn65dsi86 > *pdata, int dp_rate_idx, > goto exit; > } > > + /* For DisplayPort, disable scrambling mode. */ > + if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) > + regmap_update_bits(pdata->regmap, SN_TRAINING_SETTING_REG, > + SCRAMBLE_DISABLE, SCRAMBLE_DISABLE); > + > /* > * We'll try to link train several times. As part of link training > * the bridge chip will write DP_SET_POWER_D0 to DP_SET_POWER. If > @@ -1260,7 +1273,8 @@ static int ti_sn_bridge_probe(struct auxiliary_device > *adev, > pdata->bridge.funcs = _sn_bridge_funcs; > pdata->bridge.of_node = np; > pdata->bridge.ops = DRM_BRIDGE_OP_EDID; > - pdata->bridge.type = DRM_MODE_CONNECTOR_eDP; > + pdata->bridge.type = pdata->next_bridge->type == > DRM_MODE_CONNECTOR_DisplayPort > + ? DRM_MODE_CONNECTOR_DisplayPort : > DRM_MODE_CONNECTOR_eDP; Anyone know if there's any expectation of other types here? Or is it just as safe to do: pdata->bridge.type = pdata->next_bridge->type; To achieve the same effect? -- Kieran > > drm_bridge_add(>bridge); > > -- > 2.32.0 >
[Bug 215652] kernel 5.17-rc fail to load radeon DRM "modprobe: ERROR: could not insert 'radeon': Unknown symbol in module, or unknown parameter (see dmesg)"
https://bugzilla.kernel.org/show_bug.cgi?id=215652 --- Comment #8 from Erhard F. (erhar...@mailbox.org) --- Created attachment 300550 --> https://bugzilla.kernel.org/attachment.cgi?id=300550=edit kernel dmesg (kernel 5.17-rc7, CONFIG_DRM_RADEON=m, Talos II) Seems this is issue already fixed in -rc7. v5.17-rc7 boots on the Talos II again with radeon drm loaded from disk without an initrd or firmware being built in. Out of curiosity I'll do a bisect next week anyhow to check out which commit fixed the issue. But feel free to close here if it is not appropriate to hold this bug open any longer. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge
Am 10.03.22 um 14:03 schrieb Frieder Schrempf: > Hi Jagan, > > Am 09.03.22 um 15:01 schrieb Jagan Teki: >> Hi Frieder, >> >> On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf >> wrote: >>> >>> Hi Jagan, >>> >>> Am 03.03.22 um 17:36 schrieb Jagan Teki: Updated series about drm bridge conversion of exynos dsi. Previous version can be accessible, here [1]. Patch 1: tc358764 panel_bridge API Patch 2: connector reset Patch 3: bridge attach in MIC Patch 4: panel_bridge API Patch 5: bridge conversion Patch 6: atomic functions [1] https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fcover%2F1839data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cc99f637dd67444dfc38208da01d55963%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637824313083236643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=qF5bVwelZ35cQQygW3PvPUZkQlyFalUDsyDVjDnngiU%3Dreserved=0 Any inputs? >>> >>> Thanks for your efforts. I didn't follow the whole history, but I'm >>> looking forward and hope to see upstream support for the i.MX8MM DSIM in >>> the not too distant future. >>> >>> Can you give me a short update about the state of this patchset? Are >>> there still any major obstacles? >>> >>> I can't help with testing on Exynos, but if you have the matching >>> follow-up patches for i.MX8MM support somewhere around I could do some >>> tests with those on i.MX8MM. >> >> Unfortunately, it is getting slow due to existing exynos dsi drivers. >> Idea is to push exynos and then move the bridge as per Mailing-list >> discussion. I have initial series to support i.MX8MM on linux-next [1] >> which is working on my setup. However I'm waiting for this series to >> move further to send those on the mailing list. Indeed I'm solely >> relaying on Marek testing to move further as I too don't have Exynos >> hardware to validate. > > Thanks for the status update. Let's hope Marek or others with access to > the hardware can provide further testing. > > And thanks for providing the git tree for i.MX8MM. I will try to do some > tests on our hardware. Sorry, forgot to say that if you could cc me on future iterations of this patchset and the upcoming i.MX8MM patches, that would be great, thanks!
Re: [PATCH 1/2] dt-bindings: drm: bridge: adi,adv7533: Document adi,disable-lanes-override property
On 09/03/2022 16:11, Biju Das wrote:
> On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
> lower frequencies causes display instability. On such platforms, it
> is better to avoid switching lanes from 4 to 3 as it needs different
> set of PLL parameter constraints to make the display stable with 3
> lanes.
>
> This patch introduces 'adi,disable-lanes-override' property to disable
> lane switching at lower frequencies.
>
> Signed-off-by: Biju Das
> Reviewed-by: Lad Prabhakar
> ---
> .../devicetree/bindings/display/bridge/adi,adv7533.yaml | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> index f36209137c8a..2dc378039d21 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> @@ -84,6 +84,11 @@ properties:
>timings for HDMI output.
> type: boolean
>
> + adi,disable-lanes-override:
> +description:
> + Disables the overriding lanes at lower frequencies.
> +type: boolean
The bindings should not focus on desired feature/functionality of
driver, but hardware. You are now encoding the driver behavior in the
bindings.
Best regards,
Krzysztof
Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge
Hi Jagan, Am 09.03.22 um 15:01 schrieb Jagan Teki: > Hi Frieder, > > On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf > wrote: >> >> Hi Jagan, >> >> Am 03.03.22 um 17:36 schrieb Jagan Teki: >>> Updated series about drm bridge conversion of exynos dsi. >>> >>> Previous version can be accessible, here [1]. >>> >>> Patch 1: tc358764 panel_bridge API >>> >>> Patch 2: connector reset >>> >>> Patch 3: bridge attach in MIC >>> >>> Patch 4: panel_bridge API >>> >>> Patch 5: bridge conversion >>> >>> Patch 6: atomic functions >>> >>> [1] >>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fcover%2F1839data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cc99f637dd67444dfc38208da01d55963%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637824313083236643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=qF5bVwelZ35cQQygW3PvPUZkQlyFalUDsyDVjDnngiU%3Dreserved=0 >>> >>> Any inputs? >> >> Thanks for your efforts. I didn't follow the whole history, but I'm >> looking forward and hope to see upstream support for the i.MX8MM DSIM in >> the not too distant future. >> >> Can you give me a short update about the state of this patchset? Are >> there still any major obstacles? >> >> I can't help with testing on Exynos, but if you have the matching >> follow-up patches for i.MX8MM support somewhere around I could do some >> tests with those on i.MX8MM. > > Unfortunately, it is getting slow due to existing exynos dsi drivers. > Idea is to push exynos and then move the bridge as per Mailing-list > discussion. I have initial series to support i.MX8MM on linux-next [1] > which is working on my setup. However I'm waiting for this series to > move further to send those on the mailing list. Indeed I'm solely > relaying on Marek testing to move further as I too don't have Exynos > hardware to validate. Thanks for the status update. Let's hope Marek or others with access to the hardware can provide further testing. And thanks for providing the git tree for i.MX8MM. I will try to do some tests on our hardware. Thanks Frieder
Re: [Intel-gfx] [CI 1/2] drm/mm: Add an iterator to optimally walk over holes for an allocation (v5)
On 05/03/2022 23:36, Vivek Kasireddy wrote:
This iterator relies on drm_mm_first_hole() and drm_mm_next_hole()
functions to identify suitable holes for an allocation of a given
size by efficiently traversing the rbtree associated with the given
allocator.
It replaces the for loop in drm_mm_insert_node_in_range() and can
also be used by drm drivers to quickly identify holes of a certain
size within a given range.
v2: (Tvrtko)
- Prepend a double underscore for the newly exported first/next_hole
- s/each_best_hole/each_suitable_hole/g
- Mask out DRM_MM_INSERT_ONCE from the mode before calling
first/next_hole and elsewhere.
v3: (Tvrtko)
- Reduce the number of hunks by retaining the "mode" variable name
v4:
- Typo: s/__drm_mm_next_hole(.., hole/__drm_mm_next_hole(.., pos
v5: (Tvrtko)
- Fixed another typo: should pass caller_mode instead of mode to
the iterator in drm_mm_insert_node_in_range().
Reviewed-by: Tvrtko Ursulin
Acked-by: Christian König
Suggested-by: Tvrtko Ursulin
Signed-off-by: Vivek Kasireddy
Are we okay to merge this via drm-intel-gt-next? I haven't spotted any
in progress patches touching this area so should be conflict free.
Regards,
Tvrtko
---
drivers/gpu/drm/drm_mm.c | 32 +++-
include/drm/drm_mm.h | 36
2 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 8257f9d4f619..6ff98a0e4df3 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -352,10 +352,10 @@ static struct drm_mm_node *find_hole_addr(struct drm_mm
*mm, u64 addr, u64 size)
return node;
}
-static struct drm_mm_node *
-first_hole(struct drm_mm *mm,
- u64 start, u64 end, u64 size,
- enum drm_mm_insert_mode mode)
+struct drm_mm_node *
+__drm_mm_first_hole(struct drm_mm *mm,
+ u64 start, u64 end, u64 size,
+ enum drm_mm_insert_mode mode)
{
switch (mode) {
default:
@@ -374,6 +374,7 @@ first_hole(struct drm_mm *mm,
hole_stack);
}
}
+EXPORT_SYMBOL(__drm_mm_first_hole);
/**
* DECLARE_NEXT_HOLE_ADDR - macro to declare next hole functions
@@ -410,11 +411,11 @@ static struct drm_mm_node *name(struct drm_mm_node
*entry, u64 size) \
DECLARE_NEXT_HOLE_ADDR(next_hole_high_addr, rb_left, rb_right)
DECLARE_NEXT_HOLE_ADDR(next_hole_low_addr, rb_right, rb_left)
-static struct drm_mm_node *
-next_hole(struct drm_mm *mm,
- struct drm_mm_node *node,
- u64 size,
- enum drm_mm_insert_mode mode)
+struct drm_mm_node *
+__drm_mm_next_hole(struct drm_mm *mm,
+ struct drm_mm_node *node,
+ u64 size,
+ enum drm_mm_insert_mode mode)
{
switch (mode) {
default:
@@ -432,6 +433,7 @@ next_hole(struct drm_mm *mm,
return >hole_stack == >hole_stack ? NULL : node;
}
}
+EXPORT_SYMBOL(__drm_mm_next_hole);
/**
* drm_mm_reserve_node - insert an pre-initialized node
@@ -516,11 +518,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
u64 size, u64 alignment,
unsigned long color,
u64 range_start, u64 range_end,
- enum drm_mm_insert_mode mode)
+ enum drm_mm_insert_mode caller_mode)
{
struct drm_mm_node *hole;
u64 remainder_mask;
- bool once;
+ enum drm_mm_insert_mode mode = caller_mode & ~DRM_MM_INSERT_ONCE;
DRM_MM_BUG_ON(range_start > range_end);
@@ -533,13 +535,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
if (alignment <= 1)
alignment = 0;
- once = mode & DRM_MM_INSERT_ONCE;
- mode &= ~DRM_MM_INSERT_ONCE;
-
remainder_mask = is_power_of_2(alignment) ? alignment - 1 : 0;
- for (hole = first_hole(mm, range_start, range_end, size, mode);
-hole;
-hole = once ? NULL : next_hole(mm, hole, size, mode)) {
+ drm_mm_for_each_suitable_hole(hole, mm, range_start, range_end,
+ size, caller_mode) {
u64 hole_start = __drm_mm_hole_node_start(hole);
u64 hole_end = hole_start + hole->hole_size;
u64 adj_start, adj_end;
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index ac33ba1b18bc..dff6db627807 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -400,6 +400,42 @@ static inline u64 drm_mm_hole_node_end(const struct
drm_mm_node *hole_node)
1 : 0; \
pos = list_next_entry(pos, hole_stack))
+struct drm_mm_node *
+__drm_mm_first_hole(struct drm_mm *mm,
+ u64 start, u64 end, u64 size,
+ enum drm_mm_insert_mode mode);
+
+struct drm_mm_node *
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On 3/10/22 11:53, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 10:41:05PM +0100, Marek Vasut wrote:
On 3/8/22 17:21, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 03:47:22PM +0100, Marek Vasut wrote:
On 3/8/22 14:49, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
On 3/8/22 13:51, Maxime Ripard wrote:
On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
On 3/8/22 11:07, Jagan Teki wrote:
On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
On 3/8/22 09:03, Jagan Teki wrote:
Hi,
[...]
@@ -314,7 +321,9 @@ static const struct drm_bridge_funcs chipone_bridge_funcs =
{
static int chipone_parse_dt(struct chipone *icn)
{
struct device *dev = icn->dev;
+ struct device_node *endpoint;
struct drm_panel *panel;
+ int dsi_lanes;
int ret;
icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
@@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone *icn)
return PTR_ERR(icn->enable_gpio);
}
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+ icn->host_node = of_graph_get_remote_port_parent(endpoint);
+ of_node_put(endpoint);
+
+ if (!icn->host_node)
+ return -ENODEV;
The non-ports-based OF graph returns a -19 example on the Allwinner
Display pipeline in R16 [1].
We need to have a helper to return host_node for non-ports as I have
done it for drm_of_find_bridge.
[1] https://patchwork.amarulasolutions.com/patch/1805/
The link points to a patch marked "DO NOT MERGE", maybe that patch is
missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
required, see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
What is "non-ports-based OF graph" ?
I don't see drm_of_find_bridge() in linux-next , what is that ?
port@0 is optional as some of the DSI host OF-graph represent the
bridge or panel as child nodes instead of ports. (i think dt-binding
has to fix it to make port@0 optional)
The current upstream DT binding document says:
required:
- port@0
- port@1
So port@0 is mandatory.
In the binding, sure, but fundamentally the DT excerpt Jagan provided is
correct. If the bridge supports DCS, there's no reason to use the OF
graph in the first place: the bridge node will be a child node of the
MIPI-DSI controller (and there's no obligation to use the OF-graph for a
MIPI-DSI controller).
I believe port@0 should be made optional (or downright removed if
MIPI-DCS in the only control bus).
That's out of scope of this series anyway, so Jagan can implement patches
for that mode if needed.
Not really? You can't count on the port@0 being there generally
speaking, so you can't count on data-lanes being there either, which
exactly what you're doing in this patch.
I can because the upstream DT bindings currently say port@0 must be present,
see above. If that requirement should be relaxed, sure, but that's a
separate series.
And another upstream DT bindings say that you don't need them at all.
Which "another upstream DT bindings" do you refer to ?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
Yes, there's a conflict. Yes, it's unfortunate. But the generic DSI
binding is far more relevant than a single bridge driver.
That seems to be the wrong way around, how can generic subsystem-wide
binding take precedence over specific driver binding ?
This is the binding of the bus. You're part of that bus. You're a child
node of that bus, but nothing ever mandates that your parent node uses
the same convention. And some don't. And since your bridge can be
connected to pretty much any DSI controller, you have to use the lowest
common denominator, not make up some new constraints that not all
controller will be able to comply with.
It seems to me the ICN6211 DT bindings currently further constraint the
generic DSI bus bindings, and that seems OK to me.
Let me reiterate this again -- if someone wants to relax the
requirements currently imposed by the ICN6211 DT bindings, fine, but
that can be done in a separate patchset AND that needs DT bindings
update. Furthermore, there are no users of this ICN6211 bridge in
upstream DTs, so there is currently no bridge which would operate
without OF graph using this driver.
So figuring it out is very much a prerequisite to that series,
especially since those patches effectively make the OF-graph mandatory
in some situations, while it was purely aesthetics before.
The OF-graph is mandatory per the DT bindings of this driver. If you
implement invalid DT which does not contain port@0, it will fail DT
validation.
If this requirement should be relaxed,
[PATCH v2 7/8] drm/i915: fixup the initial fb base on DG1
The offset we get looks to be the exact start of DSM, but the
inital_plane_vma expects the address to be relative.
v2(Ville):
- The base is actually the pre-programmed GGTT address, which is then
meant to 1:1 map to somewhere inside dsm. In the case of dgpu the
base looks to just be some offset within lmem, but this also happens
to be the exact dsm start, on dg1. Therefore we should only need to
fudge the physical address, before allocating from stolen.
- Bail if it's not located in dsm.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Cc: Ville Syrjälä
---
.../drm/i915/display/intel_plane_initial.c| 21 +--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index f797fcef18fc..2aebde02ff57 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -50,7 +50,7 @@ initial_plane_vma(struct drm_i915_private *i915,
struct intel_memory_region *mem = i915->mm.stolen_region;
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
- u32 base, size;
+ u32 base, phys_base, size;
u64 pinctl;
if (!mem || plane_config->size == 0)
@@ -71,8 +71,25 @@ initial_plane_vma(struct drm_i915_private *i915,
size * 2 > i915->stolen_usable_size)
return NULL;
+ /*
+* On discrete, it looks like the GGTT base address should 1:1 map to
+* somewhere in lmem. On DG1 for some reason this intersects with the
+* exact start of DSM(possibly due to small lmem size), in which case we
+* need to allocate it directly from stolen, which means fudging the
+* physical address to be relative to the start of DSM. In such cases
+* we might also need to choose between initial fb vs fbc, if space is
+* limited.
+*/
+ phys_base = base;
+ if (IS_DG1(i915)) {
+ if (WARN_ON(phys_base < i915->dsm.start))
+ return NULL;
+
+ phys_base -= i915->dsm.start;
+ }
+
obj = i915_gem_object_create_region_at(i915->mm.stolen_region,
- base, size, 0);
+ phys_base, size, 0);
if (IS_ERR(obj))
return NULL;
--
2.34.1
[PATCH v2 8/8] drm/i915: fixup the initial fb on DG2
On DG2+ the initial fb shouldn't be placed anywhere close to DSM, and so
should just be allocated directly from LMEM.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
.../drm/i915/display/intel_plane_initial.c| 46 +++
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index 2aebde02ff57..12bda6604a1b 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -58,6 +58,31 @@ initial_plane_vma(struct drm_i915_private *i915,
base = round_down(plane_config->base,
I915_GTT_MIN_ALIGNMENT);
+ phys_base = base;
+ if (IS_DGFX(i915)) {
+ /*
+* On discrete, it looks like the GGTT base address should 1:1
+* map to somewhere in lmem. On DG1 for some reason this
+* intersects with the exact start of DSM(possibly due to small
+* lmem size), in which case we need to allocate it directly
+* from stolen, which means fudging the physical address to be
+* relative to the start of DSM. In such cases we might also
+* need to choose between initial fb vs fbc, if space is
+* limited.
+*
+* On future discrete HW, like DG2, we should be able to just
+* allocate directly from lmem it seems.
+*/
+ if (IS_DG1(i915)) {
+ if (WARN_ON(phys_base < i915->dsm.start))
+ return NULL;
+
+ phys_base -= i915->dsm.start;
+ } else {
+ mem = i915->mm.regions[INTEL_REGION_LMEM];
+ }
+ }
+
size = round_up(plane_config->base + plane_config->size,
mem->min_page_size);
size -= base;
@@ -68,28 +93,11 @@ initial_plane_vma(struct drm_i915_private *i915,
* features.
*/
if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
+ mem == i915->mm.stolen_region &&
size * 2 > i915->stolen_usable_size)
return NULL;
- /*
-* On discrete, it looks like the GGTT base address should 1:1 map to
-* somewhere in lmem. On DG1 for some reason this intersects with the
-* exact start of DSM(possibly due to small lmem size), in which case we
-* need to allocate it directly from stolen, which means fudging the
-* physical address to be relative to the start of DSM. In such cases
-* we might also need to choose between initial fb vs fbc, if space is
-* limited.
-*/
- phys_base = base;
- if (IS_DG1(i915)) {
- if (WARN_ON(phys_base < i915->dsm.start))
- return NULL;
-
- phys_base -= i915->dsm.start;
- }
-
- obj = i915_gem_object_create_region_at(i915->mm.stolen_region,
- phys_base, size, 0);
+ obj = i915_gem_object_create_region_at(mem, phys_base, size, 0);
if (IS_ERR(obj))
return NULL;
--
2.34.1
[PATCH v2 6/8] drm/i915/display: Check mappable aperture when pinning preallocated vma
From: CQ Tang When system does not have mappable aperture, ggtt->mappable_end=0. In this case if we pass PIN_MAPPABLE when pinning vma, the pinning code will return -ENOSPC. So conditionally set PIN_MAPPABLE if HAS_GMCH(). Suggested-by: Chris P Wilson Signed-off-by: CQ Tang Cc: Radhakrishna Sripada Cc: Ap Kamal Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_plane_initial.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c index 5227e5b35206..f797fcef18fc 100644 --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c @@ -51,6 +51,7 @@ initial_plane_vma(struct drm_i915_private *i915, struct drm_i915_gem_object *obj; struct i915_vma *vma; u32 base, size; + u64 pinctl; if (!mem || plane_config->size == 0) return NULL; @@ -101,7 +102,10 @@ initial_plane_vma(struct drm_i915_private *i915, if (IS_ERR(vma)) goto err_obj; - if (i915_ggtt_pin(vma, NULL, 0, PIN_MAPPABLE | PIN_OFFSET_FIXED | base)) + pinctl = PIN_GLOBAL | PIN_OFFSET_FIXED | base; + if (HAS_GMCH(i915)) + pinctl |= PIN_MAPPABLE; + if (i915_vma_pin(vma, 0, 0, pinctl)) goto err_obj; if (i915_gem_object_is_tiled(obj) && -- 2.34.1
[PATCH v2 4/8] drm/i915: add i915_gem_object_create_region_at()
Add a generic interface for allocating an object at some specific
offset, and convert stolen over. Later we will want to hook this up to
different backends.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
.../drm/i915/display/intel_plane_initial.c| 4 +-
drivers/gpu/drm/i915/gem/i915_gem_create.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_region.c| 47 ++--
drivers/gpu/drm/i915/gem/i915_gem_region.h| 7 ++
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 1 +
drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 74 ---
drivers/gpu/drm/i915/gem/i915_gem_stolen.h| 4 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 1 +
drivers/gpu/drm/i915/gt/intel_rc6.c | 8 +-
drivers/gpu/drm/i915/intel_memory_region.h| 1 +
drivers/gpu/drm/i915/selftests/mock_region.c | 1 +
12 files changed, 77 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index e207d12286b5..5227e5b35206 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -3,6 +3,7 @@
* Copyright © 2021 Intel Corporation
*/
+#include "gem/i915_gem_region.h"
#include "i915_drv.h"
#include "intel_atomic_plane.h"
#include "intel_display.h"
@@ -69,7 +70,8 @@ initial_plane_vma(struct drm_i915_private *i915,
size * 2 > i915->stolen_usable_size)
return NULL;
- obj = i915_gem_object_create_stolen_for_preallocated(i915, base, size);
+ obj = i915_gem_object_create_region_at(i915->mm.stolen_region,
+ base, size, 0);
if (IS_ERR(obj))
return NULL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c
b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index c6eb023d3d86..5802692ea604 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -123,7 +123,7 @@ __i915_gem_object_create_user_ext(struct drm_i915_private
*i915, u64 size,
*/
flags = I915_BO_ALLOC_USER;
- ret = mr->ops->init_object(mr, obj, size, 0, flags);
+ ret = mr->ops->init_object(mr, obj, I915_BO_INVALID_OFFSET, size, 0,
flags);
if (ret)
goto object_free;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c
b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index c9b2e8b91053..3428ddfb2fdb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -27,11 +27,12 @@ void i915_gem_object_release_memory_region(struct
drm_i915_gem_object *obj)
mutex_unlock(>objects.lock);
}
-struct drm_i915_gem_object *
-i915_gem_object_create_region(struct intel_memory_region *mem,
- resource_size_t size,
- resource_size_t page_size,
- unsigned int flags)
+static struct drm_i915_gem_object *
+__i915_gem_object_create_region(struct intel_memory_region *mem,
+ resource_size_t offset,
+ resource_size_t size,
+ resource_size_t page_size,
+ unsigned int flags)
{
struct drm_i915_gem_object *obj;
resource_size_t default_page_size;
@@ -86,7 +87,7 @@ i915_gem_object_create_region(struct intel_memory_region *mem,
if (default_page_size < mem->min_page_size)
flags |= I915_BO_ALLOC_PM_EARLY;
- err = mem->ops->init_object(mem, obj, size, page_size, flags);
+ err = mem->ops->init_object(mem, obj, offset, size, page_size, flags);
if (err)
goto err_object_free;
@@ -98,6 +99,40 @@ i915_gem_object_create_region(struct intel_memory_region
*mem,
return ERR_PTR(err);
}
+struct drm_i915_gem_object *
+i915_gem_object_create_region(struct intel_memory_region *mem,
+ resource_size_t size,
+ resource_size_t page_size,
+ unsigned int flags)
+{
+ return __i915_gem_object_create_region(mem, I915_BO_INVALID_OFFSET,
+ size, page_size, flags);
+}
+
+struct drm_i915_gem_object *
+i915_gem_object_create_region_at(struct intel_memory_region *mem,
+resource_size_t offset,
+resource_size_t size,
+unsigned int flags)
+{
+ GEM_BUG_ON(offset == I915_BO_INVALID_OFFSET);
+
+ if (GEM_WARN_ON(!IS_ALIGNED(size, mem->min_page_size)) ||
+ GEM_WARN_ON(!IS_ALIGNED(offset, mem->min_page_size)))
+ return ERR_PTR(-EINVAL);
+
+ if (range_overflows(offset, size, resource_size(>region)))
+ return ERR_PTR(-EINVAL);
+
+ if (!(flags &
[PATCH v2 3/8] drm/i915/stolen: consider I915_BO_ALLOC_GPU_ONLY
Keep the behaviour consistent with normal lmem, where we assume CPU access if by default required. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index 6df1600708a7..369a2a60bd7a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -695,6 +695,14 @@ static int _i915_gem_object_stolen_init(struct intel_memory_region *mem, if (size == 0) return -EINVAL; + /* +* With discrete devices, where we lack a mappable aperture there is no +* possible way to ever access this memory on the CPU side. +*/ + if (mem->type == INTEL_MEMORY_STOLEN_LOCAL && !mem->io_size && + !(flags & I915_BO_ALLOC_GPU_ONLY)) + return -ENOSPC; + stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); if (!stolen) return -ENOMEM; -- 2.34.1
[PATCH v2 5/8] drm/i915/ttm: wire up the object offset
For the ttm backend we can use existing placements fpfn and lpfn to
force the allocator to place the object at the requested offset,
potentially evicting stuff if the spot is currently occupied.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
.../gpu/drm/i915/gem/i915_gem_object_types.h | 2 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 18 ++
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 3 ++-
drivers/gpu/drm/i915/intel_region_ttm.c| 7 ++-
drivers/gpu/drm/i915/intel_region_ttm.h| 1 +
drivers/gpu/drm/i915/selftests/mock_region.c | 3 +++
6 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index fd54eb8f4826..2c88bdb8ff7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -631,6 +631,8 @@ struct drm_i915_gem_object {
struct drm_mm_node *stolen;
+ resource_size_t bo_offset;
+
unsigned long scratch;
u64 encode;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5e543ed867a2..e4a06fcf741a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -126,6 +126,8 @@ i915_ttm_select_tt_caching(const struct drm_i915_gem_object
*obj)
static void
i915_ttm_place_from_region(const struct intel_memory_region *mr,
struct ttm_place *place,
+ resource_size_t offset,
+ resource_size_t size,
unsigned int flags)
{
memset(place, 0, sizeof(*place));
@@ -133,7 +135,10 @@ i915_ttm_place_from_region(const struct
intel_memory_region *mr,
if (flags & I915_BO_ALLOC_CONTIGUOUS)
place->flags |= TTM_PL_FLAG_CONTIGUOUS;
- if (mr->io_size && mr->io_size < mr->total) {
+ if (offset != I915_BO_INVALID_OFFSET) {
+ place->fpfn = offset >> PAGE_SHIFT;
+ place->lpfn = place->fpfn + (size >> PAGE_SHIFT);
+ } else if (mr->io_size && mr->io_size < mr->total) {
if (flags & I915_BO_ALLOC_GPU_ONLY) {
place->flags |= TTM_PL_FLAG_TOPDOWN;
} else {
@@ -155,12 +160,14 @@ i915_ttm_placement_from_obj(const struct
drm_i915_gem_object *obj,
placement->num_placement = 1;
i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] :
- obj->mm.region, requested, flags);
+ obj->mm.region, requested, obj->bo_offset,
+ obj->base.size, flags);
/* Cache this on object? */
placement->num_busy_placement = num_allowed;
for (i = 0; i < placement->num_busy_placement; ++i)
- i915_ttm_place_from_region(obj->mm.placements[i], busy + i,
flags);
+ i915_ttm_place_from_region(obj->mm.placements[i], busy + i,
+ obj->bo_offset, obj->base.size,
flags);
if (num_allowed == 0) {
*busy = *requested;
@@ -802,7 +809,8 @@ static int __i915_ttm_migrate(struct drm_i915_gem_object
*obj,
struct ttm_placement placement;
int ret;
- i915_ttm_place_from_region(mr, , flags);
+ i915_ttm_place_from_region(mr, , obj->bo_offset,
+ obj->base.size, flags);
placement.num_placement = 1;
placement.num_busy_placement = 1;
placement.placement =
@@ -1159,6 +1167,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region
*mem,
drm_gem_private_object_init(>drm, >base, size);
i915_gem_object_init(obj, _gem_ttm_obj_ops, _class, flags);
+ obj->bo_offset = offset;
+
/* Don't put on a region list until we're either locked or fully
initialized. */
obj->mm.region = mem;
INIT_LIST_HEAD(>mm.region_link);
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 129f668f21ff..8e4e3f72c1ef 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -71,7 +71,8 @@ static int i915_ttm_buddy_man_alloc(struct
ttm_resource_manager *man,
GEM_BUG_ON(min_page_size < mm->chunk_size);
- if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+ if (place->fpfn + bman_res->base.num_pages != place->lpfn &&
+ place->flags & TTM_PL_FLAG_CONTIGUOUS) {
unsigned long pages;
size = roundup_pow_of_two(size);
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 737ef3f4ab54..62ff77445b01 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -12,6 +12,7 @@
[PATCH v2 1/8] drm/i915/lmem: don't treat small BAR as an error
Just pass along the probed io_size. The backend should be able to utilize the entire range here, even if some of it is non-mappable. It does leave open with what to do with stolen local-memory. Signed-off-by: Matthew Auld Cc: Thomas Hellström Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index 6cecfdae07ad..783d81072c3b 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -93,6 +93,7 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) struct intel_memory_region *mem; resource_size_t min_page_size; resource_size_t io_start; + resource_size_t io_size; resource_size_t lmem_size; int err; @@ -124,7 +125,8 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) io_start = pci_resource_start(pdev, 2); - if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2))) + io_size = min(pci_resource_len(pdev, 2), lmem_size); + if (!io_size) return ERR_PTR(-ENODEV); min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K : @@ -134,7 +136,7 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) lmem_size, min_page_size, io_start, -lmem_size, +io_size, INTEL_MEMORY_LOCAL, 0, _region_lmem_ops); -- 2.34.1
[PATCH v2 2/8] drm/i915/stolen: don't treat small BAR as an error
From: Akeem G Abodunrin
On client platforms with reduced LMEM BAR, we should be able to continue
with driver load with reduced io_size. Instead of using the BAR size to
determine the how large stolen should be, we should instead use the
ADDR_RANGE register to figure this out(at least on platforms like DG2).
For simplicity we don't attempt to support partially mappable stolen.
Signed-off-by: Akeem G Abodunrin
Co-developed-by: Matthew Auld
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 48 --
drivers/gpu/drm/i915/i915_reg.h| 3 ++
2 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 0bf8f61134af..6df1600708a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -12,6 +12,8 @@
#include "gem/i915_gem_lmem.h"
#include "gem/i915_gem_region.h"
+#include "gt/intel_gt.h"
+#include "gt/intel_region_lmem.h"
#include "i915_drv.h"
#include "i915_gem_stolen.h"
#include "i915_reg.h"
@@ -750,9 +752,9 @@ static int init_stolen_lmem(struct intel_memory_region *mem)
if (GEM_WARN_ON(resource_size(>region) == 0))
return -ENODEV;
- if (!io_mapping_init_wc(>iomap,
- mem->io_start,
- mem->io_size))
+ if (mem->io_size && !io_mapping_init_wc(>iomap,
+ mem->io_start,
+ mem->io_size))
return -EIO;
/*
@@ -773,7 +775,8 @@ static int init_stolen_lmem(struct intel_memory_region *mem)
static int release_stolen_lmem(struct intel_memory_region *mem)
{
- io_mapping_fini(>iomap);
+ if (mem->io_size)
+ io_mapping_fini(>iomap);
i915_gem_cleanup_stolen(mem->i915);
return 0;
}
@@ -790,25 +793,43 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915,
u16 type,
{
struct intel_uncore *uncore = >uncore;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+ resource_size_t dsm_size, dsm_base, lmem_size;
struct intel_memory_region *mem;
+ resource_size_t io_start, io_size;
resource_size_t min_page_size;
- resource_size_t io_start;
- resource_size_t lmem_size;
- u64 lmem_base;
- lmem_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
- if (GEM_WARN_ON(lmem_base >= pci_resource_len(pdev, 2)))
+ if (WARN_ON_ONCE(instance))
return ERR_PTR(-ENODEV);
- lmem_size = pci_resource_len(pdev, 2) - lmem_base;
- io_start = pci_resource_start(pdev, 2) + lmem_base;
+ /* Use DSM base address instead for stolen memory */
+ dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
+ if (IS_DG1(uncore->i915)) {
+ lmem_size = pci_resource_len(pdev, 2);
+ if (WARN_ON(lmem_size < dsm_base))
+ return ERR_PTR(-ENODEV);
+ } else {
+ resource_size_t lmem_range;
+
+ lmem_range = intel_gt_read_register(>gt0,
XEHPSDV_TILE0_ADDR_RANGE) & 0x;
+ lmem_size = lmem_range >> XEHPSDV_TILE_LMEM_RANGE_SHIFT;
+ lmem_size *= SZ_1G;
+ }
+
+ dsm_size = lmem_size - dsm_base;
+ if (pci_resource_len(pdev, 2) < lmem_size) {
+ io_start = 0;
+ io_size = 0;
+ } else {
+ io_start = pci_resource_start(pdev, 2) + dsm_base;
+ io_size = dsm_size;
+ }
min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
I915_GTT_PAGE_SIZE_4K;
- mem = intel_memory_region_create(i915, lmem_base, lmem_size,
+ mem = intel_memory_region_create(i915, dsm_base, dsm_size,
min_page_size,
-io_start, lmem_size,
+io_start, io_size,
type, instance,
_region_stolen_lmem_ops);
if (IS_ERR(mem))
@@ -822,6 +843,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915,
u16 type,
drm_dbg(>drm, "Stolen Local memory IO start: %pa\n",
>io_start);
+ drm_dbg(>drm, "Stolen Local DSM base: %pa\n", _base);
intel_memory_region_set_name(mem, "stolen-local");
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 25ecddc706af..0183823b4c55 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8466,6 +8466,9 @@ enum skl_power_gate {
#define SGGI_DIS REG_BIT(15)
#define SGR_DIS REG_BIT(13)
+#define XEHPSDV_TILE0_ADDR_RANGE _MMIO(0x4900)
+#define
[PATCH v2 0/8] Some more bits for small BAR enabling
The leftover bits around dealing with stolen-local memory + small BAR, plus some related fixes. v2: some tweaks based on feedback from Ville -- 2.34.1
Re: [Intel-gfx] [PATCH] drm/i915/uapi: Add DRM_I915_QUERY_GEOMETRY_SUBSLICES
On 10/03/2022 05:18, Matt Atwood wrote:
Newer platforms have DSS that aren't necessarily available for both
geometry and compute, two queries will need to exist. This introduces
the first, when passing a valid engine class and engine instance in the
flags returns a topology describing geometry.
v2: fix white space errors
Cc: Ashutosh Dixit
Cc: Matt Roper
Cc: Joonas Lahtinen
UMD (mesa): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14143
Signed-off-by: Matt Atwood
---
drivers/gpu/drm/i915/i915_query.c | 68 ++-
include/uapi/drm/i915_drm.h | 24 +++
2 files changed, 65 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 2dfbc22857a3..e4f35da28642 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -9,6 +9,7 @@
#include "i915_drv.h"
#include "i915_perf.h"
#include "i915_query.h"
+#include "gt/intel_engine_user.h"
#include
static int copy_query_item(void *query_hdr, size_t query_sz,
@@ -28,36 +29,30 @@ static int copy_query_item(void *query_hdr, size_t query_sz,
return 0;
}
-static int query_topology_info(struct drm_i915_private *dev_priv,
- struct drm_i915_query_item *query_item)
+static int fill_topology_info(const struct sseu_dev_info *sseu,
+ struct drm_i915_query_item *query_item,
+ const u8 *subslice_mask)
{
- const struct sseu_dev_info *sseu = _gt(dev_priv)->info.sseu;
struct drm_i915_query_topology_info topo;
u32 slice_length, subslice_length, eu_length, total_length;
int ret;
- if (query_item->flags != 0)
- return -EINVAL;
+ BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
if (sseu->max_slices == 0)
return -ENODEV;
- BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
-
slice_length = sizeof(sseu->slice_mask);
subslice_length = sseu->max_slices * sseu->ss_stride;
eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride;
total_length = sizeof(topo) + slice_length + subslice_length +
eu_length;
- ret = copy_query_item(, sizeof(topo), total_length,
- query_item);
+ ret = copy_query_item(, sizeof(topo), total_length, query_item);
+
if (ret != 0)
return ret;
- if (topo.flags != 0)
- return -EINVAL;
-
memset(, 0, sizeof(topo));
topo.max_slices = sseu->max_slices;
topo.max_subslices = sseu->max_subslices;
@@ -69,27 +64,61 @@ static int query_topology_info(struct drm_i915_private
*dev_priv,
topo.eu_stride = sseu->eu_stride;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
- , sizeof(topo)))
+, sizeof(topo)))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
- >slice_mask, slice_length))
+>slice_mask, slice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) + slice_length),
- sseu->subslice_mask, subslice_length))
+sizeof(topo) + slice_length),
+subslice_mask, subslice_length))
return -EFAULT;
if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
- sizeof(topo) +
- slice_length + subslice_length),
- sseu->eu_mask, eu_length))
+sizeof(topo) +
+slice_length + subslice_length),
+sseu->eu_mask, eu_length))
return -EFAULT;
return total_length;
}
+static int query_topology_info(struct drm_i915_private *dev_priv,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu = _gt(dev_priv)->info.sseu;
+
+ if (query_item->flags != 0)
+ return -EINVAL;
+
+ return fill_topology_info(sseu, query_item, sseu->subslice_mask);
+}
+
+static int query_geometry_subslices(struct drm_i915_private *i915,
+ struct drm_i915_query_item *query_item)
+{
+ const struct sseu_dev_info *sseu;
+ struct intel_engine_cs *engine;
+ u8 engine_class, engine_instance;
+
+ if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
+ return -ENODEV;
+
+ engine_class = query_item->flags & 0xFF;
+ engine_instance = (query_item->flags >> 8) & 0xFF;
+
+ engine = intel_engine_lookup_user(i915, engine_class, engine_instance);
+
Re: [PATCH] drm/vc4: add tracepoints for CL submissions
El 10/3/22 a las 12:12, Maxime Ripard escribió: On Tue, Mar 01, 2022 at 01:58:26PM -0100, Melissa Wen wrote: On 02/25, Maxime Ripard wrote: Hi Melissa, On Tue, Feb 01, 2022 at 08:26:51PM -0100, Melissa Wen wrote: Trace submit_cl_ioctl and related IRQs for CL submission and bin/render jobs execution. It might be helpful to get a rendering timeline and track job throttling. Signed-off-by: Melissa Wen I'm not really sure what to do about this patch to be honest. My understanding is that tracepoints are considered as userspace ABI, but I can't really judge whether your traces are good enough or if it's something that will bit us some time down the road. Thanks for taking a look at this patch. So, I followed the same path of tracing CL submissions on v3d. I mean, tracking submit_cl ioctl, points when a job (bin/render) starts it execution, and irqs of completion (bin/render job). We used it to examine job throttling when running Chromium and, therefore, in addition to have the timeline of jobs execution, I show some data submitted in the ioctl to make connections. I think these tracers might be useful for some investigation in the future, but I'm also not sure if all data are necessary to be displayed. Yeah, I'm sure that it's useful :) I don't see anything wrong with that patch, really. What I meant is that I don't really have the experience to judge if there's anything wrong in the first place :) If you can get someone with more experience with the v3d driver (Emma, Iago maybe?) I'll be definitely be ok merging that patch I've checked this patch and I've been using these tracepoints. They have been working properly. Reviewed-by: Jose Maria Casanova Crespo Regards, Chema Casanova
Re: [PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Implement bridge connector operations
Hi Doug,
Quoting Doug Anderson (2022-03-07 19:52:08)
> Hi,
>
> On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham
> wrote:
> >
> > From: Laurent Pinchart
> >
> > Implement the bridge connector-related .get_edid() operation, and report
> > the related bridge capabilities and type.
> >
> > Signed-off-by: Laurent Pinchart
> > Reviewed-by: Stephen Boyd
> > Reviewed-by: Douglas Anderson
> > Signed-off-by: Kieran Bingham
> > ---
> > Changes since v1:
> >
> > - The connector .get_modes() operation doesn't rely on EDID anymore,
> > __ti_sn_bridge_get_edid() and ti_sn_bridge_get_edid() got merged
> > together
> >
> > Notes from Kieran:
> >
> > RB Tags collected from:
> >
> > https://lore.kernel.org/all/20210322030128.2283-9-laurent.pinchart+rene...@ideasonboard.com/
> >
> > However this was over a year ago, so let me know if other patches now
> > superceed this one or otherwise invalidate this update.
> >
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index c55848588123..ffb6c04f6c46 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -1154,6 +1154,19 @@ static void ti_sn_bridge_post_disable(struct
> > drm_bridge *bridge)
> > pm_runtime_put_sync(pdata->dev);
> > }
> >
> > +static struct edid *ti_sn_bridge_get_edid(struct drm_bridge *bridge,
> > + struct drm_connector *connector)
> > +{
> > + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
> > + struct edid *edid;
> > +
> > + pm_runtime_get_sync(pdata->dev);
> > + edid = drm_get_edid(connector, >aux.ddc);
> > + pm_runtime_put_autosuspend(pdata->dev);
> > +
> > + return edid;
> > +}
> > +
> > static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
> > .attach = ti_sn_bridge_attach,
> > .detach = ti_sn_bridge_detach,
> > @@ -1162,6 +1175,7 @@ static const struct drm_bridge_funcs
> > ti_sn_bridge_funcs = {
> > .enable = ti_sn_bridge_enable,
> > .disable = ti_sn_bridge_disable,
> > .post_disable = ti_sn_bridge_post_disable,
> > + .get_edid = ti_sn_bridge_get_edid,
> > };
> >
> > static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
> > @@ -1248,6 +1262,8 @@ static int ti_sn_bridge_probe(struct auxiliary_device
> > *adev,
> >
> > pdata->bridge.funcs = _sn_bridge_funcs;
> > pdata->bridge.of_node = np;
> > + pdata->bridge.ops = DRM_BRIDGE_OP_EDID;
> > + pdata->bridge.type = DRM_MODE_CONNECTOR_eDP;
>
> This doesn't look right to me. In the eDP case the EDID reading is
> driven by the panel.
Now that I have the optional connector working based on Sam's series I
think this is the last issue to solve before reposting the DP/HPD
support.
Are you saying that the bridge.ops should only set DRM_BRIDGE_OP_EDID
when pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort?
--
Regards
Kieran
>
> -Doug
[Bug 215669] [drm:gfx_v10_0_priv_reg_irq [amdgpu]] *ERROR* Illegal register access in command stream
https://bugzilla.kernel.org/show_bug.cgi?id=215669 --- Comment #2 from Andreas Polnas (andreas.polna...@hotmail.com) --- Thanks Alex, I have posted it on the mesa gitlab as an issue shown below: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6113 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
Hi Maxime,
On Thu, Mar 10, 2022 at 11:57:38AM +0100, Maxime Ripard wrote:
> On Thu, Mar 10, 2022 at 12:42:12PM +0200, Laurent Pinchart wrote:
> > On Tue, Mar 08, 2022 at 01:51:40PM +0100, Maxime Ripard wrote:
> > > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > > > >
> > > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > > chipone_bridge_funcs = {
> > > > > > > >static int chipone_parse_dt(struct chipone *icn)
> > > > > > > >{
> > > > > > > > struct device *dev = icn->dev;
> > > > > > > > + struct device_node *endpoint;
> > > > > > > > struct drm_panel *panel;
> > > > > > > > + int dsi_lanes;
> > > > > > > > int ret;
> > > > > > > >
> > > > > > > > icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> > > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct
> > > > > > > > chipone *icn)
> > > > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > > > }
> > > > > > > >
> > > > > > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node,
> > > > > > > > 0, 0);
> > > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > > "data-lanes");
> > > > > > > > + icn->host_node =
> > > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > > + of_node_put(endpoint);
> > > > > > > > +
> > > > > > > > + if (!icn->host_node)
> > > > > > > > + return -ENODEV;
> > > > > > >
> > > > > > > The non-ports-based OF graph returns a -19 example on the
> > > > > > > Allwinner
> > > > > > > Display pipeline in R16 [1].
> > > > > > >
> > > > > > > We need to have a helper to return host_node for non-ports as I
> > > > > > > have
> > > > > > > done it for drm_of_find_bridge.
> > > > > > >
> > > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > > >
> > > > > > The link points to a patch marked "DO NOT MERGE", maybe that patch
> > > > > > is
> > > > > > missing the DSI host port@0 OF graph link ? Both port@0 and port@1
> > > > > > are
> > > > > > required, see:
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > > >
> > > > > > What is "non-ports-based OF graph" ?
> > > > > >
> > > > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > > > >
> > > > > port@0 is optional as some of the DSI host OF-graph represent the
> > > > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > > > has to fix it to make port@0 optional)
> > > >
> > > > The current upstream DT binding document says:
> > > >
> > > > required:
> > > > - port@0
> > > > - port@1
> > > >
> > > > So port@0 is mandatory.
> > >
> > > In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> > > correct. If the bridge supports DCS, there's no reason to use the OF
> > > graph in the first place: the bridge node will be a child node of the
> > > MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> > > MIPI-DSI controller).
> > >
> > > I believe port@0 should be made optional (or downright removed if
> > > MIPI-DCS in the only control bus).
> >
> > I think we should make ports mandatory in all cases actually.
> >
> > The DT parent-child hierarchy is meant to model control relations
> > between devices, so a DSI device controlled through DCS should be a
> > child of the DSI controller. No disagreement there.
> >
> > The OF graph is meant to model data connections. While a DSI device
> > controlled through DCS will use the same DSI link for data transfer, the
> > two concepts are different. We have taken shortcuts and decided to not
> > use OF graph for some DSI devices (not necessarily as a well thought
> > decision, it was sometimes just not considered).
>
> I disagree. Unless the data path is explicitly stated using the OF-graph
> or some other binding, it's inferred.
It is today, and for video data, I think it's showing to be a problem
:-)
> We never asked ourselves where the
> data from an i2c chip, an ethernet controller or an v4l2 output device
> was coming from. It comes from the parent bus, because it's what makes
> sense. Making a requirement on the OF-Graph to model this would create a
> big inconsistency.
I'm afraid I disagree, especially when it comes to data transfers from
device to device. The device tree has never tried to model those until
OF graph.
> > This has led to different issues that we're having to deal with today,
> > making it more difficult to develop generic code. Going forward, I
> > think new bindings should always use OF
Re: [PATCH] drm/vc4: add tracepoints for CL submissions
On Tue, Mar 01, 2022 at 01:58:26PM -0100, Melissa Wen wrote: > On 02/25, Maxime Ripard wrote: > > Hi Melissa, > > > > On Tue, Feb 01, 2022 at 08:26:51PM -0100, Melissa Wen wrote: > > > Trace submit_cl_ioctl and related IRQs for CL submission and bin/render > > > jobs execution. It might be helpful to get a rendering timeline and > > > track job throttling. > > > > > > Signed-off-by: Melissa Wen > > > > I'm not really sure what to do about this patch to be honest. > > > > My understanding is that tracepoints are considered as userspace ABI, > > but I can't really judge whether your traces are good enough or if it's > > something that will bit us some time down the road. > > Thanks for taking a look at this patch. > > So, I followed the same path of tracing CL submissions on v3d. I mean, > tracking submit_cl ioctl, points when a job (bin/render) starts it > execution, and irqs of completion (bin/render job). We used it to > examine job throttling when running Chromium and, therefore, in addition > to have the timeline of jobs execution, I show some data submitted in > the ioctl to make connections. I think these tracers might be useful for > some investigation in the future, but I'm also not sure if all data are > necessary to be displayed. Yeah, I'm sure that it's useful :) I don't see anything wrong with that patch, really. What I meant is that I don't really have the experience to judge if there's anything wrong in the first place :) If you can get someone with more experience with the v3d driver (Emma, Iago maybe?) I'll be definitely be ok merging that patch Maxime signature.asc Description: PGP signature
Re: DSI Bridge switching
On Thu, Mar 10, 2022 at 6:15 AM Adam Ford wrote: > > On Wed, Mar 9, 2022 at 1:11 PM Jagan Teki wrote: > > > > or a Hi All, > > > > On Thu, Oct 14, 2021 at 6:45 PM Jagan Teki > > wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Oct 8, 2021 at 7:07 PM Laurent Pinchart > > > wrote: > > > > > > > > Hello, > > > > > > > > On Fri, Oct 08, 2021 at 03:27:43PM +0200, Andrzej Hajda wrote: > > > > > Hi, > > > > > > > > > > Removed my invalid email (I will update files next week). > > > > > > > > > > On 08.10.2021 13:14, Jagan Teki wrote: > > > > > > Hi, > > > > > > > > > > > > I think this seems to be a known use case for industrial these days > > > > > > with i.mx8m. > > > > > > > > > > > > The host DSI would configure with two bridges one for DSI to LVDS > > > > > > (SN65DSI83) and another for DSI to HDMI Out (ADV7535). Technically > > > > > > we > > > > > > can use only one bridge at a time as host DSI support single out > > > > > > port. > > > > > > So we can have two separate device tree files for LVDS and HDMI and > > > > > > load them static. > > > > > > > > > > > > But, one of the use cases is to support both of them in single dts, > > > > > > and > > > > > > - Turn On LVDS (default) > > > > > > - Turn Off LVDS then Turn On HDMI when cable plug-in > > > > > > > > > > Are you sure it will work from hardware PoV? Do you have some demuxer? > > > > > isolation of pins? > > > > > > > > It may be in the category of "you shouldn't do this, but it actually > > > > works". I've seen the same being done with two CSI-2 camera sensors > > > > connected to the same receiver, with one of them being held in reset at > > > > all times. > > > > > > Yes. Here the design has 2 MIPI D-PHY switches. Each switch take 2 > > > input data lanes and 1 clock lane from SoC and produces 4 data lanes > > > and 2 clock lanes and from switch output 2 lanes and 1 clock are > > > inputting to HDMI bridge and other 2 lanes and 1 clock is inputting to > > > LVDS. So 1st pair of 1st switch and 1st pair of 2nd switch goes to > > > HDMI and 2nd pair of 1st switch and 2nd pair of 2nd switch does to > > > LVDS. > > > > > > However, routing of these lanes are controlled by SEL, OE GPIO pins. > > > So at a time we can access only single bridge. > > > > > > > > > > > > > The HDMI event can be detected via some HDMI-INT GPIO on-board > > > > > > design. > > > > > > > > > > > > The possible solution, I'm thinking of adding LVDS on port 1, HDMI > > > > > > on > > > > > > port 2 in the DSI host node, and trying to attach the respective > > > > > > bridge based on HDMI-INT like repeating the bridge attachment cycle > > > > > > based on the HDMI-INT. > > > > > > > > > > I think more appropriate would be to share the same port, but provide > > > > > two endpoints inside this port - we have two hardware sharing the same > > > > > physical port. > > > > > > > > That sounds like the correct DT description to me. > > > > > > > > > > Can it be possible to do bridge attachment at runtime? something > > > > > > like > > > > > > a bridge hotplug event? or any other possible solutions? > > > > > > > > > > > > Any suggestions? > > > > > > > > > > Practically it is possible, see exynos_dsi + panels, or exynos_dsi + > > > > > some toshiba bridge - panel and bridge are dynamically 'plugged' and > > > > > 'unplugged' from exynos_drm, but they do not use bridge chain for this > > > > > and some other reasons. (un|re|)plugging should be performed of course > > > > > when pipeline is off (connector disconnected). I am not sure about > > > > > bridges added to bridge chain - you need to inspect all opses to > > > > > ensure > > > > > it can be done safely. > > > > > > > > > > And the main issue: Daniel does not like it :) > > > > > > > > Neither do I :-) Could it be handled with two DRM connectors that are > > > > mutually exclusive ? > > > > > > How about adding lvds-connector, hdmi-connector on the pipeline and > > > select them based on the switch SEL GPIO? does it make sense to do > > > this implementation via display-connector.c > > > > I have somehow managed to make runtime switching possible between LVDS > > and HDMI with the help of DRM bridges. > > > > | => ADV7535=> > > HDMI-A Connector > > DSI Host => display-switch => | > > |=> SN65DSI83 => > > Panel-Simple > > > > display-switch here is a bridge driver that can switch or attach the > > downstream bridge flow based on HDMI HPD here. One potential problem > > is that when we switch from LVDS to HDMI Out the HDMI Out is displayed > > with the resolution that LVDS has and it is unable to display higher > > HDMI resolutions even though it supports it. Does anyone aware of > > changing the resolution at runtime, please let me know? > > > > Technically, the display-switch hardware does available in various forms > > 1. MIPI Switch PI3WVR626 > > 2. Conventional Mux Switch > > 3. Converter bridge DSI to
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Thu, Mar 10, 2022 at 12:42:12PM +0200, Laurent Pinchart wrote:
> Hi Maxime,
>
> On Tue, Mar 08, 2022 at 01:51:40PM +0100, Maxime Ripard wrote:
> > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > > >
> > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > [...]
> > > > >
> > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > chipone_bridge_funcs = {
> > > > > > >static int chipone_parse_dt(struct chipone *icn)
> > > > > > >{
> > > > > > > struct device *dev = icn->dev;
> > > > > > > + struct device_node *endpoint;
> > > > > > > struct drm_panel *panel;
> > > > > > > + int dsi_lanes;
> > > > > > > int ret;
> > > > > > >
> > > > > > > icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone
> > > > > > > *icn)
> > > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > > }
> > > > > > >
> > > > > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0,
> > > > > > > 0);
> > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > "data-lanes");
> > > > > > > + icn->host_node =
> > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > + of_node_put(endpoint);
> > > > > > > +
> > > > > > > + if (!icn->host_node)
> > > > > > > + return -ENODEV;
> > > > > >
> > > > > > The non-ports-based OF graph returns a -19 example on the Allwinner
> > > > > > Display pipeline in R16 [1].
> > > > > >
> > > > > > We need to have a helper to return host_node for non-ports as I have
> > > > > > done it for drm_of_find_bridge.
> > > > > >
> > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > >
> > > > > The link points to a patch marked "DO NOT MERGE", maybe that patch is
> > > > > missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
> > > > > required, see:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > >
> > > > > What is "non-ports-based OF graph" ?
> > > > >
> > > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > > >
> > > > port@0 is optional as some of the DSI host OF-graph represent the
> > > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > > has to fix it to make port@0 optional)
> > >
> > > The current upstream DT binding document says:
> > >
> > > required:
> > > - port@0
> > > - port@1
> > >
> > > So port@0 is mandatory.
> >
> > In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> > correct. If the bridge supports DCS, there's no reason to use the OF
> > graph in the first place: the bridge node will be a child node of the
> > MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> > MIPI-DSI controller).
> >
> > I believe port@0 should be made optional (or downright removed if
> > MIPI-DCS in the only control bus).
>
> I think we should make ports mandatory in all cases actually.
>
> The DT parent-child hierarchy is meant to model control relations
> between devices, so a DSI device controlled through DCS should be a
> child of the DSI controller. No disagreement there.
>
> The OF graph is meant to model data connections. While a DSI device
> controlled through DCS will use the same DSI link for data transfer, the
> two concepts are different. We have taken shortcuts and decided to not
> use OF graph for some DSI devices (not necessarily as a well thought
> decision, it was sometimes just not considered).
I disagree. Unless the data path is explicitly stated using the OF-graph
or some other binding, it's inferred. We never asked ourselves where the
data from an i2c chip, an ethernet controller or an v4l2 output device
was coming from. It comes from the parent bus, because it's what makes
sense. Making a requirement on the OF-Graph to model this would create a
big inconsistency.
> This has led to different issues that we're having to deal with today,
> making it more difficult to develop generic code. Going forward, I
> think new bindings should always use OF graph to model the data
> connection.
Either way, that discussion is irrelevant. Not all DSI controllers use
OF-Graph, a bridge can be attached to any of them, so we can't require
OF-Graph support in any bridge.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
On Tue, Mar 08, 2022 at 10:41:05PM +0100, Marek Vasut wrote:
> On 3/8/22 17:21, Maxime Ripard wrote:
> > On Tue, Mar 08, 2022 at 03:47:22PM +0100, Marek Vasut wrote:
> > > On 3/8/22 14:49, Maxime Ripard wrote:
> > > > On Tue, Mar 08, 2022 at 02:27:40PM +0100, Marek Vasut wrote:
> > > > > On 3/8/22 13:51, Maxime Ripard wrote:
> > > > > > On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > > > > > > On 3/8/22 11:07, Jagan Teki wrote:
> > > > > > > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > [...]
> > > > > > > > >
> > > > > > > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > > > > > > chipone_bridge_funcs = {
> > > > > > > > > > > static int chipone_parse_dt(struct chipone *icn)
> > > > > > > > > > > {
> > > > > > > > > > > struct device *dev = icn->dev;
> > > > > > > > > > > + struct device_node *endpoint;
> > > > > > > > > > > struct drm_panel *panel;
> > > > > > > > > > > + int dsi_lanes;
> > > > > > > > > > > int ret;
> > > > > > > > > > >
> > > > > > > > > > > icn->vdd1 = devm_regulator_get_optional(dev,
> > > > > > > > > > > "vdd1");
> > > > > > > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct
> > > > > > > > > > > chipone *icn)
> > > > > > > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > > > > > > }
> > > > > > > > > > >
> > > > > > > > > > > + endpoint =
> > > > > > > > > > > of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
> > > > > > > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > > > > > > "data-lanes");
> > > > > > > > > > > + icn->host_node =
> > > > > > > > > > > of_graph_get_remote_port_parent(endpoint);
> > > > > > > > > > > + of_node_put(endpoint);
> > > > > > > > > > > +
> > > > > > > > > > > + if (!icn->host_node)
> > > > > > > > > > > + return -ENODEV;
> > > > > > > > > >
> > > > > > > > > > The non-ports-based OF graph returns a -19 example on the
> > > > > > > > > > Allwinner
> > > > > > > > > > Display pipeline in R16 [1].
> > > > > > > > > >
> > > > > > > > > > We need to have a helper to return host_node for non-ports
> > > > > > > > > > as I have
> > > > > > > > > > done it for drm_of_find_bridge.
> > > > > > > > > >
> > > > > > > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > > > > > > >
> > > > > > > > > The link points to a patch marked "DO NOT MERGE", maybe that
> > > > > > > > > patch is
> > > > > > > > > missing the DSI host port@0 OF graph link ? Both port@0 and
> > > > > > > > > port@1 are
> > > > > > > > > required, see:
> > > > > > > > >
> > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > > > > > > >
> > > > > > > > > What is "non-ports-based OF graph" ?
> > > > > > > > >
> > > > > > > > > I don't see drm_of_find_bridge() in linux-next , what is that
> > > > > > > > > ?
> > > > > > > >
> > > > > > > > port@0 is optional as some of the DSI host OF-graph represent
> > > > > > > > the
> > > > > > > > bridge or panel as child nodes instead of ports. (i think
> > > > > > > > dt-binding
> > > > > > > > has to fix it to make port@0 optional)
> > > > > > >
> > > > > > > The current upstream DT binding document says:
> > > > > > >
> > > > > > >required:
> > > > > > > - port@0
> > > > > > > - port@1
> > > > > > >
> > > > > > > So port@0 is mandatory.
> > > > > >
> > > > > > In the binding, sure, but fundamentally the DT excerpt Jagan
> > > > > > provided is
> > > > > > correct. If the bridge supports DCS, there's no reason to use the OF
> > > > > > graph in the first place: the bridge node will be a child node of
> > > > > > the
> > > > > > MIPI-DSI controller (and there's no obligation to use the OF-graph
> > > > > > for a
> > > > > > MIPI-DSI controller).
> > > > > >
> > > > > > I believe port@0 should be made optional (or downright removed if
> > > > > > MIPI-DCS in the only control bus).
> > > > >
> > > > > That's out of scope of this series anyway, so Jagan can implement
> > > > > patches
> > > > > for that mode if needed.
> > > >
> > > > Not really? You can't count on the port@0 being there generally
> > > > speaking, so you can't count on data-lanes being there either, which
> > > > exactly what you're doing in this patch.
> > >
> > > I can because the upstream DT bindings currently say port@0 must be
> > > present,
> > > see above. If that requirement should be relaxed, sure, but that's a
> > > separate series.
> >
> > And another upstream DT bindings say that you don't need them at all.
>
> Which "another upstream DT bindings" do you refer to ?
Re: [PATCH v14 22/22] arm64: dts: mt8195: add display node for vdosys1
Il 10/03/22 04:55, Nancy.Lin ha scritto:
Add display node for vdosys1.
Signed-off-by: Nancy.Lin
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 223 +++
1 file changed, 223 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index dbca699bba05..e650ec759235 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
..snip..
+
+ ethdr0: ethdr@1c114000 {
+ compatible = "mediatek,mt8195-disp-ethdr";
+ reg = <0 0x1c114000 0 0x1000>,
+ <0 0x1c115000 0 0x1000>,
+ <0 0x1c117000 0 0x1000>,
+ <0 0x1c119000 0 0x1000>,
+ <0 0x1c11A000 0 0x1000>,
+ <0 0x1c11B000 0 0x1000>,
+ <0 0x1c11C000 0 0x1000>;
Hello Nancy,
looks like you partially forgot to use lower-case hex here...
0x1c11a000 0x1c11b000 0x1c11c000
+ reg-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0",
"gfx_fe1",
+ "vdo_be", "adl_ds";
+ mediatek,gce-client-reg = < SUBSYS_1c11 0x4000
0x1000>,
+ < SUBSYS_1c11 0x5000
0x1000>,
+ < SUBSYS_1c11 0x7000
0x1000>,
+ < SUBSYS_1c11 0x9000
0x1000>,
+ < SUBSYS_1c11 0xA000
0x1000>,
+ < SUBSYS_1c11 0xB000
0x1000>,
+ < SUBSYS_1c11 0xC000
0x1000>;
...and here too: 0xa000 0xb000 0xc000
Please fix that, after which, you can add my
Reviewed-by: AngeloGioacchino Del Regno
+ clocks = < CLK_VDO1_DISP_MIXER>,
+< CLK_VDO1_HDR_VDO_FE0>,
+< CLK_VDO1_HDR_VDO_FE1>,
+< CLK_VDO1_HDR_GFX_FE0>,
+< CLK_VDO1_HDR_GFX_FE1>,
+< CLK_VDO1_HDR_VDO_BE>,
+< CLK_VDO1_26M_SLOW>,
+< CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
+< CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
+< CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
+< CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
+< CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
+< CLK_TOP_ETHDR>;
+ clock-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0",
"gfx_fe1",
+ "vdo_be", "adl_ds", "vdo_fe0_async",
"vdo_fe1_async",
+ "gfx_fe0_async",
"gfx_fe1_async","vdo_be_async",
+ "ethdr_top";
+ power-domains = < MT8195_POWER_DOMAIN_VDOSYS1>;
+ iommus = <_vpp M4U_PORT_L3_HDR_DS>,
+<_vpp M4U_PORT_L3_HDR_ADL>;
+ interrupts = ; /*
disp mixer */
+ resets = <
MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
+<
MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
+<
MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
+<
MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
+<
MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
+ };
+
};
};
Re: [PATCH v14 17/22] drm/mediatek: add ovl_adaptor support for MT8195
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add ovl_adaptor driver for MT8195. Ovl_adaptor is an encapsulated module and designed for simplified DRM control flow. This module is composed of 8 RDMAs, 4 MERGEs and an ETHDR. Two RDMAs merge into one layer, so this module support 4 layers. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/Makefile | 1 + drivers/gpu/drm/mediatek/mtk_disp_drv.h | 17 + .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 443 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.h| 1 + 4 files changed, 462 insertions(+) create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
Re: [PATCH v14 16/22] drm/mediatek: add mediatek-drm plane color encoding info
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add plane color encoding information for color space conversion. It's a preparation for adding support for mt8195 ovl_adaptor mdp_rdma csc control. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 1 + drivers/gpu/drm/mediatek/mtk_drm_plane.h | 1 + 2 files changed, 2 insertions(+)
Re: [PATCH v14 15/22] drm/mediatek: add ETHDR support for MT8195
Il 10/03/22 04:55, Nancy.Lin ha scritto: ETHDR is a part of ovl_adaptor. ETHDR is designed for HDR video and graphics conversion in the external display path. It handles multiple HDR input types and performs tone mapping, color space/color format conversion, and then combine different layers, output the required HDR or SDR signal to the subsequent display path. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/Makefile | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + drivers/gpu/drm/mediatek/mtk_ethdr.c | 376 + drivers/gpu/drm/mediatek/mtk_ethdr.h | 23 ++ 5 files changed, 402 insertions(+) create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.c create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.h
Re: [PATCH v14 14/22] drm/mediatek: add display merge async reset control
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add merge async reset control in mtk_merge_stop. Async hw doesn't do self reset on each sof signal(start of frame), so need to reset the async to clear the hw status for the next merge start. Signed-off-by: Nancy.Lin Reviewed-by: CK Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/mtk_disp_merge.c | 4 1 file changed, 4 insertions(+)
Re: [PATCH v14 13/22] drm/mediatek: add display merge mute/unmute support for MT8195
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add merge mute/unmute setting for MT8195. MT8195 Vdosys1 merge1~merge4 support HW mute function. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/mtk_disp_merge.c | 13 + 1 file changed, 13 insertions(+)
Re: [PATCH v14 12/22] drm/mediatek: add display merge start/stop API for cmdq support
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add merge start/stop API for cmdq support. The ovl_adaptor merges are configured with each drm plane update. Need to enable/disable merge with cmdq making sure all the settings taken effect in the same vblank. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 2 ++ drivers/gpu/drm/mediatek/mtk_disp_merge.c | 20 +--- 2 files changed, 19 insertions(+), 3 deletions(-)
Re: [PATCH v14 11/22] drm/mediatek: add display merge advance config API for MT8195
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add merge new advance config API. The original merge API is mtk_ddp_comp_funcs function prototype. The API interface parameters cannot be modified, so add a new config API for extension. This is the preparation for ovl_adaptor merge control. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 ++ drivers/gpu/drm/mediatek/mtk_disp_merge.c | 52 --- 2 files changed, 48 insertions(+), 7 deletions(-)
Re: [PATCH v14 10/22] drm/mediatek: add display MDP RDMA support for MT8195
Il 10/03/22 04:55, Nancy.Lin ha scritto: Add MDP_RDMA driver for MT8195. MDP_RDMA is the DMA engine of the ovl_adaptor component. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- drivers/gpu/drm/mediatek/Makefile | 3 +- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 7 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 315 drivers/gpu/drm/mediatek/mtk_mdp_rdma.h | 20 ++ 6 files changed, 346 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.c create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.h
Re: [PATCH v14 03/22] dt-bindings: mediatek: add ethdr definition for mt8195
Il 10/03/22 04:54, Nancy.Lin ha scritto: Add vdosys1 ETHDR definition. Signed-off-by: Nancy.Lin Reviewed-by: Chun-Kuang Hu Reviewed-by: AngeloGioacchino Del Regno --- .../display/mediatek/mediatek,ethdr.yaml | 158 ++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
Re: [PATCH v14 08/22] soc: mediatek: change the mutex defines and the mutex_mod type
Il 10/03/22 04:55, Nancy.Lin ha scritto: This is a preparation for adding support for mt8195 vdosys1 mutex. The vdosys1 path component contains ovl_adaptor, merge5, and dp_intf1. Ovl_adaptor is composed of several sub-elements, so change it to support multi-bit control. Signed-off-by: Nancy.Lin Reviewed-by: AngeloGioacchino Del Regno --- drivers/soc/mediatek/mtk-mutex.c | 264 +++ 1 file changed, 132 insertions(+), 132 deletions(-)
Re: [v4 5/5] drm/edid: check for HF-SCDB block
On Wed, Mar 02, 2022 at 05:35:11PM +0800, Lee Shawn C wrote:
> Find HF-SCDB information in CEA extensions block. And retrieve
> Max_TMDS_Character_Rate that support by sink device.
>
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Ankit Nautiyal
> Signed-off-by: Lee Shawn C
> ---
> drivers/gpu/drm/drm_edid.c | 36
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 2b8ddc956ce2..d6b48c543c23 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3350,6 +3350,7 @@ add_detailed_modes(struct drm_connector *connector,
> struct edid *edid,
> #define EXT_VIDEO_DATA_BLOCK_420 0x0E
> #define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
> #define EXT_VIDEO_HF_EEODB_DATA_BLOCK0x78
> +#define EXT_VIDEO_HF_SCDB_DATA_BLOCK 0x79
> #define EDID_BASIC_AUDIO (1 << 6)
> #define EDID_CEA_YCRCB444(1 << 5)
> #define EDID_CEA_YCRCB422(1 << 4)
> @@ -4277,6 +4278,20 @@ static bool cea_db_is_vcdb(const u8 *db)
> return true;
> }
>
> +static bool cea_db_is_hf_scdb(const u8 *db)
> +{
> + if (cea_db_tag(db) != USE_EXTENDED_TAG)
> + return false;
> +
> + if (cea_db_payload_len(db) < 7)
> + return false;
> +
> + if (cea_db_extended_tag(db) != EXT_VIDEO_HF_SCDB_DATA_BLOCK)
> + return false;
> +
> + return true;
> +}
> +
> static bool cea_db_is_y420cmdb(const u8 *db)
> {
> if (cea_db_tag(db) != USE_EXTENDED_TAG)
> @@ -4987,6 +5002,25 @@ static void drm_parse_vcdb(struct drm_connector
> *connector, const u8 *db)
> info->rgb_quant_range_selectable = true;
> }
>
> +static void drm_parse_hf_scdb(struct drm_connector *connector, const u8 *db)
> +{
> + struct drm_display_info *info = >display_info;
> + u32 max_tmds_clock;
> +
> + DRM_DEBUG_KMS("HF-SCDB version 0x%02x\n", db[4]);
> +
> + max_tmds_clock = db[5] * 5000;
> + if (info->max_tmds_clock < max_tmds_clock) {
> + info->max_tmds_clock = max_tmds_clock;
> + DRM_DEBUG_KMS("HF-SCDB: max TMDS clock %d kHz\n",
> + info->max_tmds_clock);
> + }
> +
> + /*
> + * ToDo: Parse the remaining SCDB data if needed
> + */
If I'm reading the spec right this block should contain the exact same
stuff as the HF-VSDB. We should reuse the same code for parsing both.
> +}
> +
> static
> void drm_get_max_frl_rate(int max_frl_rate, u8 *max_lanes, u8
> *max_rate_per_lane)
> {
> @@ -5282,6 +5316,8 @@ static void drm_parse_cea_ext(struct drm_connector
> *connector,
> drm_parse_y420cmdb_bitmap(connector, db);
> if (cea_db_is_vcdb(db))
> drm_parse_vcdb(connector, db);
> + if (cea_db_is_hf_scdb(db))
> + drm_parse_hf_scdb(connector, db);
> if (cea_db_is_hdmi_hdr_metadata_block(db))
> drm_parse_hdr_metadata_block(connector, db);
> }
> --
> 2.17.1
--
Ville Syrjälä
Intel
Re: [v4 2/5] drm/edid: parse multiple CEA extension block
On Wed, Mar 02, 2022 at 05:35:08PM +0800, Lee Shawn C wrote:
> Try to find and parse more CEA ext blocks if edid->extensions
> is greater than one.
>
> v2: split prvious patch to two. And do CEA block parsing
> in this one.
> v3: simplify this patch based on previous change.
>
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Ankit Nautiyal
> Signed-off-by: Lee Shawn C
> ---
> drivers/gpu/drm/drm_edid.c | 36 ++--
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 375e70d9de86..c4a47465ba76 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4319,16 +4319,24 @@ static void drm_parse_y420cmdb_bitmap(struct
> drm_connector *connector,
> static int
> add_cea_modes(struct drm_connector *connector, struct edid *edid)
> {
> - const u8 *cea, *db, *hdmi = NULL, *video = NULL;
> - u8 dbl, hdmi_len, video_len = 0;
> + const u8 *cea, *db;
> + u8 dbl, hdmi_len;
> int modes = 0, ext_index = 0;
> + int i, start, end;
I think everything here apart from modes and ext_index can
be moved into the loop.
Apart from that 1-2 look fine to me. intel-gfx wasn't cc:d however
so we have no ci results for any of this.
>
> - cea = drm_find_cea_extension(edid, _index);
> - if (cea && cea_revision(cea) >= 3) {
> - int i, start, end;
> + for (;;) {
> + const u8 *hdmi = NULL, *video = NULL;
> + u8 video_len = 0;
> +
> + cea = drm_find_cea_extension(edid, _index);
> + if (!cea)
> + break;
> +
> + if (cea_revision(cea) < 3)
> + continue;
>
> if (cea_db_offsets(cea, , ))
> - return 0;
> + continue;
>
> for_each_cea_db(cea, i, start, end) {
> db = [i];
> @@ -4350,15 +4358,15 @@ add_cea_modes(struct drm_connector *connector, struct
> edid *edid)
> dbl - 1);
> }
> }
> - }
>
> - /*
> - * We parse the HDMI VSDB after having added the cea modes as we will
> - * be patching their flags when the sink supports stereo 3D.
> - */
> - if (hdmi)
> - modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
> - video_len);
> + /*
> + * We parse the HDMI VSDB after having added the cea modes as
> we will
> + * be patching their flags when the sink supports stereo 3D.
> + */
> + if (hdmi)
> + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len,
> video,
> + video_len);
> + }
>
> return modes;
> }
> --
> 2.17.1
--
Ville Syrjälä
Intel
Re: [PATCH V3 05/13] drm: bridge: icn6211: Add DSI lane count DT property parsing
Hi Maxime,
On Tue, Mar 08, 2022 at 01:51:40PM +0100, Maxime Ripard wrote:
> On Tue, Mar 08, 2022 at 11:29:59AM +0100, Marek Vasut wrote:
> > On 3/8/22 11:07, Jagan Teki wrote:
> > > On Tue, Mar 8, 2022 at 3:19 PM Marek Vasut wrote:
> > > >
> > > > On 3/8/22 09:03, Jagan Teki wrote:
> > > >
> > > > Hi,
> > > >
> > > > [...]
> > > >
> > > > > > @@ -314,7 +321,9 @@ static const struct drm_bridge_funcs
> > > > > > chipone_bridge_funcs = {
> > > > > >static int chipone_parse_dt(struct chipone *icn)
> > > > > >{
> > > > > > struct device *dev = icn->dev;
> > > > > > + struct device_node *endpoint;
> > > > > > struct drm_panel *panel;
> > > > > > + int dsi_lanes;
> > > > > > int ret;
> > > > > >
> > > > > > icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
> > > > > > @@ -350,15 +359,42 @@ static int chipone_parse_dt(struct chipone
> > > > > > *icn)
> > > > > > return PTR_ERR(icn->enable_gpio);
> > > > > > }
> > > > > >
> > > > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0,
> > > > > > 0);
> > > > > > + dsi_lanes = of_property_count_u32_elems(endpoint,
> > > > > > "data-lanes");
> > > > > > + icn->host_node = of_graph_get_remote_port_parent(endpoint);
> > > > > > + of_node_put(endpoint);
> > > > > > +
> > > > > > + if (!icn->host_node)
> > > > > > + return -ENODEV;
> > > > >
> > > > > The non-ports-based OF graph returns a -19 example on the Allwinner
> > > > > Display pipeline in R16 [1].
> > > > >
> > > > > We need to have a helper to return host_node for non-ports as I have
> > > > > done it for drm_of_find_bridge.
> > > > >
> > > > > [1] https://patchwork.amarulasolutions.com/patch/1805/
> > > >
> > > > The link points to a patch marked "DO NOT MERGE", maybe that patch is
> > > > missing the DSI host port@0 OF graph link ? Both port@0 and port@1 are
> > > > required, see:
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/bridge/chipone,icn6211.yaml#n53
> > > >
> > > > What is "non-ports-based OF graph" ?
> > > >
> > > > I don't see drm_of_find_bridge() in linux-next , what is that ?
> > >
> > > port@0 is optional as some of the DSI host OF-graph represent the
> > > bridge or panel as child nodes instead of ports. (i think dt-binding
> > > has to fix it to make port@0 optional)
> >
> > The current upstream DT binding document says:
> >
> > required:
> > - port@0
> > - port@1
> >
> > So port@0 is mandatory.
>
> In the binding, sure, but fundamentally the DT excerpt Jagan provided is
> correct. If the bridge supports DCS, there's no reason to use the OF
> graph in the first place: the bridge node will be a child node of the
> MIPI-DSI controller (and there's no obligation to use the OF-graph for a
> MIPI-DSI controller).
>
> I believe port@0 should be made optional (or downright removed if
> MIPI-DCS in the only control bus).
I think we should make ports mandatory in all cases actually.
The DT parent-child hierarchy is meant to model control relations
between devices, so a DSI device controlled through DCS should be a
child of the DSI controller. No disagreement there.
The OF graph is meant to model data connections. While a DSI device
controlled through DCS will use the same DSI link for data transfer, the
two concepts are different. We have taken shortcuts and decided to not
use OF graph for some DSI devices (not necessarily as a well thought
decision, it was sometimes just not considered). This has led to
different issues that we're having to deal with today, making it more
difficult to develop generic code. Going forward, I think new bindings
should always use OF graph to model the data connection.
--
Regards,
Laurent Pinchart
Re: DSI Bridge switching
On Wed, Mar 09, 2022 at 06:45:10PM -0600, Adam Ford wrote: > On Wed, Mar 9, 2022 at 1:11 PM Jagan Teki wrote: > > > > or a Hi All, > > > > On Thu, Oct 14, 2021 at 6:45 PM Jagan Teki > > wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Oct 8, 2021 at 7:07 PM Laurent Pinchart > > > wrote: > > > > > > > > Hello, > > > > > > > > On Fri, Oct 08, 2021 at 03:27:43PM +0200, Andrzej Hajda wrote: > > > > > Hi, > > > > > > > > > > Removed my invalid email (I will update files next week). > > > > > > > > > > On 08.10.2021 13:14, Jagan Teki wrote: > > > > > > Hi, > > > > > > > > > > > > I think this seems to be a known use case for industrial these days > > > > > > with i.mx8m. > > > > > > > > > > > > The host DSI would configure with two bridges one for DSI to LVDS > > > > > > (SN65DSI83) and another for DSI to HDMI Out (ADV7535). Technically > > > > > > we > > > > > > can use only one bridge at a time as host DSI support single out > > > > > > port. > > > > > > So we can have two separate device tree files for LVDS and HDMI and > > > > > > load them static. > > > > > > > > > > > > But, one of the use cases is to support both of them in single dts, > > > > > > and > > > > > > - Turn On LVDS (default) > > > > > > - Turn Off LVDS then Turn On HDMI when cable plug-in > > > > > > > > > > Are you sure it will work from hardware PoV? Do you have some demuxer? > > > > > isolation of pins? > > > > > > > > It may be in the category of "you shouldn't do this, but it actually > > > > works". I've seen the same being done with two CSI-2 camera sensors > > > > connected to the same receiver, with one of them being held in reset at > > > > all times. > > > > > > Yes. Here the design has 2 MIPI D-PHY switches. Each switch take 2 > > > input data lanes and 1 clock lane from SoC and produces 4 data lanes > > > and 2 clock lanes and from switch output 2 lanes and 1 clock are > > > inputting to HDMI bridge and other 2 lanes and 1 clock is inputting to > > > LVDS. So 1st pair of 1st switch and 1st pair of 2nd switch goes to > > > HDMI and 2nd pair of 1st switch and 2nd pair of 2nd switch does to > > > LVDS. > > > > > > However, routing of these lanes are controlled by SEL, OE GPIO pins. > > > So at a time we can access only single bridge. > > > > > > > > > > > > > The HDMI event can be detected via some HDMI-INT GPIO on-board > > > > > > design. > > > > > > > > > > > > The possible solution, I'm thinking of adding LVDS on port 1, HDMI > > > > > > on > > > > > > port 2 in the DSI host node, and trying to attach the respective > > > > > > bridge based on HDMI-INT like repeating the bridge attachment cycle > > > > > > based on the HDMI-INT. > > > > > > > > > > I think more appropriate would be to share the same port, but provide > > > > > two endpoints inside this port - we have two hardware sharing the same > > > > > physical port. > > > > > > > > That sounds like the correct DT description to me. > > > > > > > > > > Can it be possible to do bridge attachment at runtime? something > > > > > > like > > > > > > a bridge hotplug event? or any other possible solutions? > > > > > > > > > > > > Any suggestions? > > > > > > > > > > Practically it is possible, see exynos_dsi + panels, or exynos_dsi + > > > > > some toshiba bridge - panel and bridge are dynamically 'plugged' and > > > > > 'unplugged' from exynos_drm, but they do not use bridge chain for this > > > > > and some other reasons. (un|re|)plugging should be performed of course > > > > > when pipeline is off (connector disconnected). I am not sure about > > > > > bridges added to bridge chain - you need to inspect all opses to > > > > > ensure > > > > > it can be done safely. > > > > > > > > > > And the main issue: Daniel does not like it :) > > > > > > > > Neither do I :-) Could it be handled with two DRM connectors that are > > > > mutually exclusive ? > > > > > > How about adding lvds-connector, hdmi-connector on the pipeline and > > > select them based on the switch SEL GPIO? does it make sense to do > > > this implementation via display-connector.c > > > > I have somehow managed to make runtime switching possible between LVDS > > and HDMI with the help of DRM bridges. > > > > | => ADV7535=> > > HDMI-A Connector > > DSI Host => display-switch => | > > |=> SN65DSI83 => > > Panel-Simple > > > > display-switch here is a bridge driver that can switch or attach the > > downstream bridge flow based on HDMI HPD here. One potential problem > > is that when we switch from LVDS to HDMI Out the HDMI Out is displayed > > with the resolution that LVDS has and it is unable to display higher > > HDMI resolutions even though it supports it. Does anyone aware of > > changing the resolution at runtime, please let me know? > > > > Technically, the display-switch hardware does available in various forms > > 1. MIPI Switch PI3WVR626 > > 2. Conventional Mux Switch > > 3. Converter bridge
[PULL] drm-intel-fixes
Hi Dave, Daniel, One PSR2 fix for the next release candidate, if there will be one. Regards, Tvrtko drm-intel-fixes-2022-03-10: - Fix PSR2 when selective fetch is enabled and cursor at (-1, -1) (Jouni Högander) The following changes since commit ffb217a13a2eaf6d5bd974fc83036a53ca69f1e2: Linux 5.17-rc7 (2022-03-06 14:28:31 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-03-10 for you to fetch changes up to 804f468853179b9b58af05c153c411931aa5b310: drm/i915/psr: Set "SF Partial Frame Enable" also on full update (2022-03-07 14:45:31 +) - Fix PSR2 when selective fetch is enabled and cursor at (-1, -1) (Jouni Högander) Jouni Högander (1): drm/i915/psr: Set "SF Partial Frame Enable" also on full update drivers/gpu/drm/i915/display/intel_psr.c | 16 ++-- drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-)
Re: [PATCH v2 1/2] drm: Add GPU reset sysfs event
Am 09.03.22 um 19:12 schrieb Rob Clark: On Tue, Mar 8, 2022 at 11:40 PM Shashank Sharma wrote: From: Shashank Sharma This patch adds a new sysfs event, which will indicate the userland about a GPU reset, and can also provide some information like: - process ID of the process involved with the GPU reset - process name of the involved process - the GPU status info (using flags) This patch also introduces the first flag of the flags bitmap, which can be appended as and when required. Why invent something new, rather than using the already existing devcoredump? Yeah, that's a really valid question. I don't think we need (or should encourage/allow) something drm specific when there is already an existing solution used by both drm and non-drm drivers. Userspace should not have to learn to support yet another mechanism to do the same thing. Question is how is userspace notified about new available core dumps? Regards, Christian. BR, -R
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915: Improve long running OCL w/a for GuC submission
On 09/03/2022 21:16, John Harrison wrote: On 3/8/2022 01:41, Tvrtko Ursulin wrote: On 03/03/2022 22:37, john.c.harri...@intel.com wrote: From: John Harrison A workaround was added to the driver to allow OpenCL workloads to run 'forever' by disabling pre-emption on the RCS engine for Gen12. It is not totally unbound as the heartbeat will kick in eventually and cause a reset of the hung engine. However, this does not work well in GuC submission mode. In GuC mode, the pre-emption timeout is how GuC detects hung contexts and triggers a per engine reset. Thus, disabling the timeout means also losing all per engine reset ability. A full GT reset will still occur when the heartbeat finally expires, but that is a much more destructive and undesirable mechanism. The purpose of the workaround is actually to give OpenCL tasks longer to reach a pre-emption point after a pre-emption request has been issued. This is necessary because Gen12 does not support mid-thread pre-emption and OpenCL can have long running threads. So, rather than disabling the timeout completely, just set it to a 'long' value. v2: Review feedback from Tvrtko - must hard code the 'long' value instead of determining it algorithmically. So make it an extra CONFIG definition. Also, remove the execlist centric comment from the existing pre-emption timeout CONFIG option given that it applies to more than just execlists. Signed-off-by: John Harrison Reviewed-by: Daniele Ceraolo Spurio (v1) Acked-by: Michal Mrozek --- drivers/gpu/drm/i915/Kconfig.profile | 26 +++ drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 ++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile index 39328567c200..7cc38d25ee5c 100644 --- a/drivers/gpu/drm/i915/Kconfig.profile +++ b/drivers/gpu/drm/i915/Kconfig.profile @@ -57,10 +57,28 @@ config DRM_I915_PREEMPT_TIMEOUT default 640 # milliseconds help How long to wait (in milliseconds) for a preemption event to occur - when submitting a new context via execlists. If the current context - does not hit an arbitration point and yield to HW before the timer - expires, the HW will be reset to allow the more important context - to execute. + when submitting a new context. If the current context does not hit + an arbitration point and yield to HW before the timer expires, the + HW will be reset to allow the more important context to execute. + + This is adjustable via + /sys/class/drm/card?/engine/*/preempt_timeout_ms + + May be 0 to disable the timeout. + + The compiled in default may get overridden at driver probe time on + certain platforms and certain engines which will be reflected in the + sysfs control. + +config DRM_I915_PREEMPT_TIMEOUT_COMPUTE + int "Preempt timeout for compute engines (ms, jiffy granularity)" + default 7500 # milliseconds + help + How long to wait (in milliseconds) for a preemption event to occur + when submitting a new context to a compute capable engine. If the + current context does not hit an arbitration point and yield to HW + before the timer expires, the HW will be reset to allow the more + important context to execute. This is adjustable via /sys/class/drm/card?/engine/*/preempt_timeout_ms diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 4185c7338581..cc0954ad836a 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -438,9 +438,14 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id, engine->props.timeslice_duration_ms = CONFIG_DRM_I915_TIMESLICE_DURATION; - /* Override to uninterruptible for OpenCL workloads. */ + /* + * Mid-thread pre-emption is not available in Gen12. Unfortunately, + * some OpenCL workloads run quite long threads. That means they get + * reset due to not pre-empting in a timely manner. So, bump the + * pre-emption timeout value to be much higher for compute engines. + */ if (GRAPHICS_VER(i915) == 12 && (engine->flags & I915_ENGINE_HAS_RCS_REG_STATE)) - engine->props.preempt_timeout_ms = 0; + engine->props.preempt_timeout_ms = CONFIG_DRM_I915_PREEMPT_TIMEOUT_COMPUTE; I wouldn't go as far as adding a config option since as it is it only applies to Gen12 but Kconfig text says nothing about that. And I am not saying you should add a Gen12 specific config option, that would be weird. So IMO just drop it. You were the one arguing that the driver was illegally overriding the user's explicitly chosen settings, including the compile time config This is a bit out of context and illegally don't think used, so misrepresents the earlier discussion. And I certainly did not suggest a kconfig option.
[PATCH] drm/bridge: anx7625: Fix not correct get property counts
The property length which returns from "of_get_property", divided by
sizeof(int) to get the total property counts.
Fixes: fd0310b6fe7d ("drm/bridge: anx7625: add MIPI DPI input feature")
Signed-off-by: Xin Ji
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index c6a9a02ed762..87081d5b408d 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1594,6 +1594,7 @@ static int anx7625_get_swing_setting(struct device *dev,
if (of_get_property(dev->of_node,
"analogix,lane0-swing", _regs)) {
+ num_regs /= sizeof(int);
if (num_regs > DP_TX_SWING_REG_CNT)
num_regs = DP_TX_SWING_REG_CNT;
@@ -1604,6 +1605,7 @@ static int anx7625_get_swing_setting(struct device *dev,
if (of_get_property(dev->of_node,
"analogix,lane1-swing", _regs)) {
+ num_regs /= sizeof(int);
if (num_regs > DP_TX_SWING_REG_CNT)
num_regs = DP_TX_SWING_REG_CNT;
--
2.25.1
Re: [PATCH] drm: bridge: it66121: Added it66121 chip external screen status judgment
I tested it on Loongarch and MIPS, and the results were fine。 --Original-- From: "NeilArmstrong"
