Add initial support for slimport anx7625
Subsystems (4)
v7 cover Add initial support for slimport anx7625
Hi all,
The following series add initial support for the Slimport ANX7625 transmitter, a ultra-low power Full-HD 4K MIPI to DP transmitter designed for portable device.
This is the initial version, any mistakes, please let me know, I will fix it in the next series.
Thanks,
Xin
Xin Ji (2):
dt-bindings: drm/bridge: anx7625: MIPI to DP transmitter binding drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
.../bindings/display/bridge/anx7625.yaml | 91 + drivers/gpu/drm/bridge/Makefile | 2 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 + drivers/gpu/drm/bridge/analogix/Makefile | 1 + drivers/gpu/drm/bridge/analogix/anx7625.c | 2172 ++++++++++++++++++++ drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++++ 6 files changed, 2681 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/anx7625.yaml create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
Signature · 1 line
2.7.4
PATCH v7 1/2 dt-bindings: drm/bridge: anx7625: MIPI to DP transmitter binding
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed for portable device. It converts MIPI to DisplayPort 1.3 4K.
You can add support to your board with binding.
Example:
anx7625_bridge: encoder@58 {
compatible = "analogix,anx7625";
reg = <0x58>;
status = "okay";
panel-flags = <1>;
enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>;
reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
anx_1_in: endpoint {
remote-endpoint = <&mipi_dsi>;
};
};
port@2 {
reg = <2>;
anx_1_out: endpoint {
remote-endpoint = <&panel_in>;
};
};
};--- .../bindings/display/bridge/anx7625.yaml | 91 ++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/anx7625.yaml
Documentation/devicetree/bindings/display/bridge/anx7625.yaml new file +91 -0
@@ -0,0 +1,91 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2019 Analogix Semiconductor, Inc. +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/display/bridge/anx7625.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Analogix ANX7625 SlimPort (4K Mobile HD Transmitter) + +maintainers: + - Xin Ji <[email protected]> + +description: | + The ANX7625 is an ultra-low power 4K Mobile HD Transmitter + designed for portable devices. + +properties: + "#address-cells": true + "#size-cells": true + + compatible: + items: + - const: analogix,anx7625 + + reg: + maxItems: 1 + + panel-flags: + description: indicate the panel is internal or external. + maxItems: 1 + + interrupts: + maxItems: 1 + + enable-gpios: + description: used for power on chip control, POWER_EN pin D2. + maxItems: 1 + + reset-gpios: + description: used for reset chip control, RESET_N pin B7. + maxItems: 1 + + port@0: + type: object + description: + A port node pointing to MIPI DSI host port node. + + port@1: + type: object + description: + A port node pointing to MIPI DPI host port node. + + port@2: + type: object + description: + A port node pointing to panel port node. + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + - port@0 + - port@2 + +example: + - | + anx7625_bridge: encoder@58 { + compatible = "analogix,anx7625"; + reg = <0x58>; + status = "okay"; + panel-flags = <1>; + enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>; + reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>; + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + anx_1_in: endpoint { + remote-endpoint = <&mipi_dsi>; + }; + }; + + port@2 { + reg = <2>; + anx_1_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + };
Signature · 1 line
2.7.4
PATCH v7 2/2 drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
In reply to Xin JiThe ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
--- drivers/gpu/drm/bridge/Makefile | 2 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 + drivers/gpu/drm/bridge/analogix/Makefile | 1 + drivers/gpu/drm/bridge/analogix/anx7625.c | 2172 +++++++++++++++++++++++++++++ drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++++++ 5 files changed, 2590 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
drivers/gpu/drm/bridge/Makefile +1 -1
@@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-y += analogix/ obj-y += synopsys/
drivers/gpu/drm/bridge/analogix/Kconfig +6 -0
@@ -2,3 +2,9 @@ config DRM_ANALOGIX_DP tristate depends on DRM + +config ANALOGIX_ANX7625 + tristate "Analogix MIPI to DP interface support" + help + ANX7625 is an ultra-low power 4K mobile HD transmitter designed + for portable devices. It converts MIPI/DPI to DisplayPort1.3 4K.
drivers/gpu/drm/bridge/analogix/Makefile +1 -0
@@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
drivers/gpu/drm/bridge/analogix/anx7625.c new file +2172 -0
2,172 lines not on this page · raw message
drivers/gpu/drm/bridge/analogix/anx7625.h new file +410 -0
410 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Hi,
Just commenting on the mode_fixup function that was added in v7.
Quoted 22 lines · Xin Ji
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
Signed-off-by Xin Ji <[email protected]>--- drivers/gpu/drm/bridge/Makefile | 2 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 + drivers/gpu/drm/bridge/analogix/Makefile | 1 + drivers/gpu/drm/bridge/analogix/anx7625.c | 2172 +++++++++++++++++++++++++++++ drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++++++ 5 files changed, 2590 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.hdrivers/gpu/drm/bridge/Makefile +0
-0
[snip]
drivers/gpu/drm/bridge/Makefile
Quoted 13 lines
drivers/gpu/drm/bridge/Makefile +13
-0+static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + struct drm_display_mode *adj) +{ + struct anx7625_data *ctx = bridge_to_anx7625(bridge); + struct device *dev = &ctx->client->dev; + u32 hsync, hfp, hbp, hactive, hblanking; + u32 adj_hsync, adj_hfp, adj_hbp, adj_hblanking, delta_adj; + u32 vref, adj_clock; + + DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); + + mutex_lock(&ctx->lock);
Why do you need this lock?
drivers/gpu/drm/bridge/Makefile
Quoted 2 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ + hactive = mode->hdisplay;
This is never used, drop it?
Quoted 12 lines
drivers/gpu/drm/bridge/Makefile +12
-0+ hsync = mode->hsync_end - mode->hsync_start; + hfp = mode->hsync_start - mode->hdisplay; + hbp = mode->htotal - mode->hsync_end; + hblanking = mode->htotal - mode->hdisplay; + + DRM_DEV_DEBUG_DRIVER(dev, "before mode fixup\n"); + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n", + hsync, + hfp, + hbp, + adj->clock); + DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d),hsync_end(%d),htotal(%d)\n",
drivers/gpu/drm/bridge/Makefile
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile +10
-0+ adj->hsync_start, + adj->hsync_end, + adj->htotal); + + adj_hfp = hfp; + adj_hsync = hsync; + adj_hbp = hbp; + adj_hblanking = hblanking; + + /* plus 1 if hfp is odd */
A better way to word these comments is to say "hfp needs to be even", otherwise, you're just repeating what we can already see in the code.
drivers/gpu/drm/bridge/Makefile
Quoted 2 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ if (hfp & 0x1) { + adj_hfp = hfp + 1;
adj_hfp -= 1 for consistency?
drivers/gpu/drm/bridge/Makefile
Quoted 6 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ adj_hblanking += 1; + } + + /* minus 1 if hbp is odd */ + if (hbp & 0x1) { + adj_hbp = hbp - 1;
ditto, adj_hbp -= 1;
drivers/gpu/drm/bridge/Makefile
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile +7
-0+ adj_hblanking -= 1; + } + + /* plus 1 if hsync is odd */ + if (hsync & 0x1) { + if (adj_hblanking < hblanking) + adj_hsync = hsync + 1;
ditto
drivers/gpu/drm/bridge/Makefile
Quoted 2 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ else + adj_hsync = hsync - 1;
ditto
drivers/gpu/drm/bridge/Makefile
Quoted 6 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ } + + /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {
should this be adj_hblanking/adj_hfp/adj_hbp?
Quoted 6 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;
drivers/gpu/drm/bridge/Makefile
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);
How likely is it that this mode is going to work? Can you just return false here to reject the mode?
drivers/gpu/drm/bridge/Makefile
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile +7
-0+ } else if (adj_hfp < HP_MIN) { + /* adjust hfp if hfp less than HP_MIN */ + delta_adj = HP_MIN - adj_hfp; + adj_hfp = HP_MIN; + + /* + * balance total HBlanking pixel, if HBP hasn't enough space,
"does not have enough space"
Quoted 1 line
+ * adjust HSYNC length, otherwize adjust HBP
otherwise
Quoted 13 lines
drivers/gpu/drm/bridge/Makefile +13
-0+ */ + if ((adj_hbp - delta_adj) < HP_MIN) + /* hbp not enough space */ + adj_hsync -= delta_adj; + else + adj_hbp -= delta_adj; + } else if (adj_hbp < HP_MIN) { + delta_adj = HP_MIN - adj_hbp; + adj_hbp = HP_MIN; + + /* + * balance total HBlanking pixel, if HBP hasn't enough space, + * adjust HSYNC length, otherwize adjust HBP
drivers/gpu/drm/bridge/Makefile
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile +10
-0+ */ + if ((adj_hfp - delta_adj) < HP_MIN) + /* hbp not enough space */ + adj_hsync -= delta_adj; + else + adj_hfp -= delta_adj; + } + + DRM_DEV_DEBUG_DRIVER(dev, "after mode fixup\n"); + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n",
Add spaces after commas in your debug strings (same above and below).
drivers/gpu/drm/bridge/Makefile
Quoted 4 lines
drivers/gpu/drm/bridge/Makefile +4
-0+ adj_hsync, + adj_hfp, + adj_hbp, + adj->clock);
Put these 4 on a single line.
Quoted 6 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ + /* reconstruct timing */ + adj->hsync_start = adj->hdisplay + adj_hfp; + adj->hsync_end = adj->hsync_start + adj_hsync; + adj->htotal = adj->hsync_end + adj_hbp; + DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d),hsync_end(%d),htotal(%d)\n",
drivers/gpu/drm/bridge/Makefile
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile +9
-0+ adj->hsync_start, + adj->hsync_end, + adj->htotal); + + mutex_unlock(&ctx->lock); + + return true; +} +
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 28 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.Quoted 22 lines · Xin Ji
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
Signed-off-by Xin Ji <[email protected]>--- drivers/gpu/drm/bridge/Makefile | 2 +- drivers/gpu/drm/bridge/analogix/Kconfig | 6 + drivers/gpu/drm/bridge/analogix/Makefile | 1 + drivers/gpu/drm/bridge/analogix/anx7625.c | 2172 +++++++++++++++++++++++++++++ drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++++++ 5 files changed, 2590 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.hdrivers/gpu/drm/bridge/Makefile +0
-0[snip]
Quoted 15 lines
drivers/gpu/drm/bridge/Makefile
Quoted 13 lines
drivers/gpu/drm/bridge/Makefile +13
-0+static bool anx7625_bridge_mode_fixup(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + struct drm_display_mode *adj) +{ + struct anx7625_data *ctx = bridge_to_anx7625(bridge); + struct device *dev = &ctx->client->dev; + u32 hsync, hfp, hbp, hactive, hblanking; + u32 adj_hsync, adj_hfp, adj_hbp, adj_hblanking, delta_adj; + u32 vref, adj_clock; + + DRM_DEV_DEBUG_DRIVER(dev, "drm mode fixup set\n"); + + mutex_lock(&ctx->lock);Why do you need this lock?
Seems no need this lock, I'll remove it.
Quoted 5 lines
drivers/gpu/drm/bridge/Makefile
Quoted 3 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ + hactive = mode->hdisplay;This is never used, drop it?
OK, I'll drop it.
Quoted 18 lines
Quoted 18 lines
drivers/gpu/drm/bridge/Makefile +17
-0+ hsync = mode->hsync_end - mode->hsync_start; + hfp = mode->hsync_start - mode->hdisplay; + hbp = mode->htotal - mode->hsync_end; + hblanking = mode->htotal - mode->hdisplay; + + DRM_DEV_DEBUG_DRIVER(dev, "before mode fixup\n"); + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n", + hsync, + hfp, + hbp, + adj->clock); + DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d),hsync_end(%d),htotal(%d)\n", + adj->hsync_start, + adj->hsync_end, + adj->htotal); + + adj_hfp = hfp;
Quoted 8 lines
drivers/gpu/drm/bridge/Makefile
Quoted 5 lines
drivers/gpu/drm/bridge/Makefile +5
-0+ adj_hsync = hsync; + adj_hbp = hbp; + adj_hblanking = hblanking; + + /* plus 1 if hfp is odd */A better way to word these comments is to say "hfp needs to be even", otherwise, you're just repeating what we can already see in the code.
OK
Quoted 5 lines
drivers/gpu/drm/bridge/Makefile
Quoted 3 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ if (hfp & 0x1) { + adj_hfp = hfp + 1;
adj_hfp-= 1 for consistency?
OK
Quoted 9 lines
drivers/gpu/drm/bridge/Makefile
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ adj_hblanking += 1; + } + + /* minus 1 if hbp is odd */ + if (hbp & 0x1) { + adj_hbp = hbp - 1;ditto,
adj_hbp-= 1;
OK
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile
Quoted 8 lines
drivers/gpu/drm/bridge/Makefile +7
-0+ adj_hblanking -= 1; + } + + /* plus 1 if hsync is odd */ + if (hsync & 0x1) { + if (adj_hblanking < hblanking) + adj_hsync = hsync + 1;ditto
OK
Quoted 5 lines
drivers/gpu/drm/bridge/Makefile
Quoted 3 lines
drivers/gpu/drm/bridge/Makefile +2
-0+ else + adj_hsync = hsync - 1;ditto
OK
Quoted 9 lines
drivers/gpu/drm/bridge/Makefile
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ } + + /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to adj_hsync, adj_hfp, adj_hbp.
Quoted 18 lines
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;drivers/gpu/drm/bridge/Makefile
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);
Quoted 2 lines
How likely is it that this mode is going to work? Can you just return false here to reject the mode?
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Quoted 10 lines
drivers/gpu/drm/bridge/Makefile
Quoted 8 lines
drivers/gpu/drm/bridge/Makefile +7
-0+ } else if (adj_hfp < HP_MIN) { + /* adjust hfp if hfp less than HP_MIN */ + delta_adj = HP_MIN - adj_hfp; + adj_hfp = HP_MIN; + + /* + * balance total HBlanking pixel, if HBP hasn't enough space,"does not have enough space"
OK
Quoted 4 lines
Quoted 2 lines
+ * adjust HSYNC length, otherwize adjust HBP
otherwise
OK
Quoted 18 lines
Quoted 18 lines
drivers/gpu/drm/bridge/Makefile +17
-0+ */ + if ((adj_hbp - delta_adj) < HP_MIN) + /* hbp not enough space */ + adj_hsync -= delta_adj; + else + adj_hbp -= delta_adj; + } else if (adj_hbp < HP_MIN) { + delta_adj = HP_MIN - adj_hbp; + adj_hbp = HP_MIN; + + /* + * balance total HBlanking pixel, if HBP hasn't enough space, + * adjust HSYNC length, otherwize adjust HBP + */ + if ((adj_hfp - delta_adj) < HP_MIN) + /* hbp not enough space */ + adj_hsync -= delta_adj;
Quoted 8 lines
drivers/gpu/drm/bridge/Makefile
Quoted 6 lines
drivers/gpu/drm/bridge/Makefile +6
-0+ else + adj_hfp -= delta_adj; + } + + DRM_DEV_DEBUG_DRIVER(dev, "after mode fixup\n"); + DRM_DEV_DEBUG_DRIVER(dev, "hsync(%d),hfp(%d),hbp(%d),clock(%d)\n",Add spaces after commas in your debug strings (same above and below).
OK
Quoted 7 lines
drivers/gpu/drm/bridge/Makefile
Quoted 5 lines
drivers/gpu/drm/bridge/Makefile +4
-0+ adj_hsync, + adj_hfp, + adj_hbp, + adj->clock);Put these 4 on a single line.
OK
Quoted 17 lines
Quoted 17 lines
drivers/gpu/drm/bridge/Makefile +15
-0+ + /* reconstruct timing */ + adj->hsync_start = adj->hdisplay + adj_hfp; + adj->hsync_end = adj->hsync_start + adj_hsync; + adj->htotal = adj->hsync_end + adj_hbp; + DRM_DEV_DEBUG_DRIVER(dev, "hsync_start(%d),hsync_end(%d),htotal(%d)\n", + adj->hsync_start, + adj->hsync_end, + adj->htotal); + + mutex_unlock(&ctx->lock); + + return true; +} +
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 33 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
drivers/gpu/drm/bridge/analogix/anx7625.c line 1807
Quoted 4 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +4
-0+/* +* once illegal timing detected, use default HFP, HSYNC, HBP +*/ +if (hblanking <HBLANKING_MIN || (hfp <HP_MIN && hbp <HP_MIN )) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 18 lines
Quoted 17 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +16
-0+ adj_hsync =SYNC_LEN_DEF ; + adj_hfp =HFP_HBP_DEF ; + adj_hbp =HFP_HBP_DEF ; + vref = adj->clock *1000 / (adj->htotal * adj->vtotal); +if (hblanking <HBLANKING_MIN ) { + delta_adj =HBLANKING_MIN - hblanking; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock +=DIV_ROUND_UP (adj_clock,1000 ); + }else { + delta_adj = hblanking -HBLANKING_MIN ; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -=DIV_ROUND_UP (adj_clock,1000 ); + } + +DRM_WARN ("illegal hblanking timing, use default.\n" ); +DRM_WARN ("hfp(%d),hbp(%d),hsync(%d).\n" , hfp, hbp, hsync);
Quoted 4 lines
Quoted 2 lines
How likely is it that this mode is going to work? Can you just return false here to reject the mode?
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
drivers/gpu/drm/bridge/analogix/anx7625.c line 1807
Quoted 4 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +4
-0+/* +* once illegal timing detected, use default HFP, HSYNC, HBP +*/ +if (hblanking <HBLANKING_MIN || (hfp <HP_MIN && hbp <HP_MIN )) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +6
-0+ adj_hsync =SYNC_LEN_DEF ; + adj_hfp =HFP_HBP_DEF ; + adj_hbp =HFP_HBP_DEF ; + vref = adj->clock *1000 / (adj->htotal * adj->vtotal); +if (hblanking <HBLANKING_MIN ) { + delta_adj =HBLANKING_MIN - hblanking;drivers/gpu/drm/bridge/analogix/anx7625.c line 1823
Quoted 10 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock +=DIV_ROUND_UP (adj_clock,1000 ); + }else { + delta_adj = hblanking -HBLANKING_MIN ; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -=DIV_ROUND_UP (adj_clock,1000 ); + } + +DRM_WARN ("illegal hblanking timing, use default.\n" ); +DRM_WARN ("hfp(%d),hbp(%d),hsync(%d).\n" , hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah mode_fixup is supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
drivers/gpu/drm/bridge/analogix/anx7625.c line 1807
Quoted 4 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +4
-0+/* +* once illegal timing detected, use default HFP, HSYNC, HBP +*/ +if (hblanking <HBLANKING_MIN || (hfp <HP_MIN && hbp <HP_MIN )) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +6
-0+ adj_hsync =SYNC_LEN_DEF ; + adj_hfp =HFP_HBP_DEF ; + adj_hbp =HFP_HBP_DEF ; + vref = adj->clock *1000 / (adj->htotal * adj->vtotal); +if (hblanking <HBLANKING_MIN ) { + delta_adj =HBLANKING_MIN - hblanking;drivers/gpu/drm/bridge/analogix/anx7625.c line 1823
Quoted 10 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock +=DIV_ROUND_UP (adj_clock,1000 ); + }else { + delta_adj = hblanking -HBLANKING_MIN ; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -=DIV_ROUND_UP (adj_clock,1000 ); + } + +DRM_WARN ("illegal hblanking timing, use default.\n" ); +DRM_WARN ("hfp(%d),hbp(%d),hsync(%d).\n" , hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Quoted 5 lines
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 53 lines · Xin Ji
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
drivers/gpu/drm/bridge/analogix/anx7625.c line 1807
Quoted 4 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +4
-0+/* +* once illegal timing detected, use default HFP, HSYNC, HBP +*/ +if (hblanking <HBLANKING_MIN || (hfp <HP_MIN && hbp <HP_MIN )) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +6
-0+ adj_hsync =SYNC_LEN_DEF ; + adj_hfp =HFP_HBP_DEF ; + adj_hbp =HFP_HBP_DEF ; + vref = adj->clock *1000 / (adj->htotal * adj->vtotal); +if (hblanking <HBLANKING_MIN ) { + delta_adj =HBLANKING_MIN - hblanking;drivers/gpu/drm/bridge/analogix/anx7625.c line 1823
Quoted 10 lines
drivers/gpu/drm/bridge/analogix/anx7625.c new file +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock +=DIV_ROUND_UP (adj_clock,1000 ); + }else { + delta_adj = hblanking -HBLANKING_MIN ; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -=DIV_ROUND_UP (adj_clock,1000 ); + } + +DRM_WARN ("illegal hblanking timing, use default.\n" ); +DRM_WARN ("hfp(%d),hbp(%d),hsync(%d).\n" , hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.
Quoted 5 lines
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Are we talking about external DP screen here, or some built-in panel? For the later case we do a lot of mode adjusting in many drivers ...
I haven't checked, by if our connector type is eDP then this should be all fine.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 60 lines · Daniel Vetter
Quoted 53 lines · Xin Ji
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
(quoted diff) +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;Review
Quoted 10 lines
(quoted diff) +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.Quoted 5 lines
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Quoted 5 lines
Are we talking about external DP screen here, or some built-in panel? For the later case we do a lot of mode adjusting in many drivers ...
I haven't checked, by if our connector type is eDP then this should be all fine.
Ok I read the patch now, you seem to support both. Would it work if we make this adjustement conditional on it being an internal panel only? I think that would be perfect.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Hi Daniel,
Quoted 64 lines · Daniel Vetter
Quoted 63 lines · Daniel Vetter
Quoted 53 lines · Xin Ji
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
(quoted diff) +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;Review
Quoted 10 lines
(quoted diff) +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.Quoted 5 lines
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Are we talking about external DP screen here, or some built-in panel? For the later case we do a lot of mode adjusting in many drivers ...
Quoted 10 lines
Quoted 2 lines
I haven't checked, by if our connector type is eDP then this should be all fine.
Ok I read the patch now, you seem to support both. Would it work if we make this adjustement conditional on it being an internal panel only? I think that would be perfect.
-DanielSignature · 2 lines
Daniel Vetter Software Engineer, Intel Corporation
Based on comments of V8, only keeped eDP built-in panel in V9 version, removed external DP screen support.
Quoted 1 line
Daniel Vetter
Acked-byRe: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 77 lines · Xin Ji
Hi Daniel,
Quoted 64 lines · Daniel Vetter
Quoted 63 lines · Daniel Vetter
Quoted 53 lines · Xin Ji
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
(quoted diff) +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;Review
Quoted 10 lines
(quoted diff) +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.Quoted 5 lines
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Are we talking about external DP screen here, or some built-in panel? For the later case we do a lot of mode adjusting in many drivers ...
Quoted 10 lines
Quoted 2 lines
I haven't checked, by if our connector type is eDP then this should be all fine.
Ok I read the patch now, you seem to support both. Would it work if we make this adjustement conditional on it being an internal panel only? I think that would be perfect.
-DanielSignature · 2 lines
Daniel Vetter Software Engineer, Intel Corporation
Quoted 2 lines
Based on comments of V8, only keeped eDP built-in panel in V9 version, removed external DP screen support.
Ah even better. Then the above adjusting has my:
Maybe add a comment to the code summarizing the discussion. Definitely needs to be covered in the commit message.
Thanks, Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Quoted 81 lines · Daniel Vetter
Quoted 77 lines · Xin Ji
Hi Daniel,
Quoted 64 lines · Daniel Vetter
Quoted 63 lines · Daniel Vetter
Quoted 53 lines · Xin Ji
Quoted 52 lines · Daniel Vetter
Quoted 36 lines · Nicolas Boichat
Quoted 35 lines · Xin Ji
Quoted 4 lines · Nicolas Boichat
Hi,
Just commenting on the
mode_fixupfunction that was added in v7.[snip]
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ /* + * once illegal timing detected, use default HFP, HSYNC, HBP + */ + if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {should this be adj_hblanking/adj_hfp/adj_hbp?
NO, need check original HFP and HBP, if they are not legal, driver need set default value to
adj_hsync,adj_hfp,adj_hbp.Quoted 20 lines
Quoted 7 lines
(quoted diff) +6
-0+ adj_hsync = SYNC_LEN_DEF; + adj_hfp = HFP_HBP_DEF; + adj_hbp = HFP_HBP_DEF; + vref = adj->clock * 1000 / (adj->htotal * adj->vtotal); + if (hblanking < HBLANKING_MIN) { + delta_adj = HBLANKING_MIN - hblanking;Review
Quoted 10 lines
(quoted diff) +10
-0+ adj_clock = vref * delta_adj * adj->vtotal; + adj->clock += DIV_ROUND_UP(adj_clock, 1000); + } else { + delta_adj = hblanking - HBLANKING_MIN; + adj_clock = vref * delta_adj * adj->vtotal; + adj->clock -= DIV_ROUND_UP(adj_clock, 1000); + } + + DRM_WARN("illegal hblanking timing, use default.\n"); + DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);How likely is it that this mode is going to work? Can you just return false here to reject the mode?
Quoted 7 lines
Quoted 2 lines
We want to set the default minimal Hblancking value, then it may display, otherwise. If we just return false, there is no display for sure.
Right, understand your argument. I'm pondering if it's not just better to reject the mode rather than trying a timing that is definitely quite different from what the monitor was asking for. No super strong opinion, I'll let other people on the list weigh in.
Yeah
mode_fixupis supposed to be used to adjust the mode in intermediate stages (e.g. if you go from progressive to interlaced only at the end of your pipeline or something like that). It's not meant for adjusting the mode yout actually put out through a hdmi or dp connector. For fixed panels adjusting modes to fit the panel is also fairly common, but not for external outputs.Quoted 5 lines
Quoted 2 lines
Since this is a DP bridge I'd say no adjusting, just reject what doesn't fit.
We have found some panel which HBP less than 8, if we reject to adjust video timing, then there is no display. The customer does not accept it, they push us to fix it, the only resolve way is to adjust timing.
Are we talking about external DP screen here, or some built-in panel? For the later case we do a lot of mode adjusting in many drivers ...
Quoted 10 lines
Quoted 2 lines
I haven't checked, by if our connector type is eDP then this should be all fine.
Ok I read the patch now, you seem to support both. Would it work if we make this adjustement conditional on it being an internal panel only? I think that would be perfect.
-DanielSignature · 2 lines
Daniel Vetter Software Engineer, Intel CorporationQuoted 2 lines
Based on comments of V8, only keeped eDP built-in panel in V9 version, removed external DP screen support.
Quoted 6 lines
Ah even better. Then the above adjusting has my:
Acked-by Daniel Vetter <[email protected]>Maybe add a comment to the code summarizing the discussion. Definitely needs to be covered in the commit message.
OK, I'll add it in the next serial.
Thanks, Xin
Quoted 6 lines
Thanks, Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Quoted 6 lines · Xin Ji
Hi all,
The following series add initial support for the Slimport ANX7625 transmitter, a ultra-low power Full-HD 4K MIPI to DP transmitter designed for portable device.
This is the initial version, any mistakes, please let me know, I will fix it in
^^^^^^^^^^^^^^^
Quoted 1 line
the next series.
This is actually the v7 version, but the patch zero cover letter hasn't been updated. :P The last time anyone responded to these patches was to point out three simple bugs which you fixed in v4 last November.
What changed in this version? My guess is that nothing changed and you are just prodding us to re-review it... Feel free to say that also because we can't read your mind.
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 6 lines · Dan Carpenter
Quoted 5 lines · Xin Ji
Hi all,
The following series add initial support for the Slimport ANX7625 transmitter, a ultra-low power Full-HD 4K MIPI to DP transmitter designed for portable device.
Quoted 7 lines
Quoted 1 line
This is the initial version, any mistakes, please let me know, I will fix it in
^^^^^^^^^^^^^^^Quoted 1 line
the next series.
This is actually the v7 version, but the patch zero cover letter hasn't been updated. :P The last time anyone responded to these patches was to point out three simple bugs which you fixed in v4 last November.
Sorry, I missed it, I'll resend the cover letter.
Quoted 4 lines
What changed in this version? My guess is that nothing changed and you are just prodding us to re-review it... Feel free to say that also because we can't read your mind.
I added a new bridge interface "mode_fixup" for processing critical timing. And add MIPI RX tolerance by setting register 0x1B to 0x3D.
Quoted 3 lines
regards,
dan carpenter