Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new plugin to update Algoltek DisplayPort devices #7179

Merged
merged 1 commit into from
May 3, 2024

Conversation

hughsie
Copy link
Member

@hughsie hughsie commented Apr 26, 2024

Type of pull request:

Copy link
Collaborator

@YiJaneWu YiJaneWu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed.

@hughsie
Copy link
Member Author

hughsie commented Apr 29, 2024

@YiJaneWu did the update still work? Did I break anything?

@hughsie hughsie requested a review from superm1 April 29, 2024 07:05
@YiJaneWu
Copy link
Collaborator

YiJaneWu commented Apr 29, 2024

Hi Richard,
I’m so appreciate that you help me to modify the algoltek-aux plugin, they can run normally in our computer.

But there is only one problem that needs to be solved.

Our original payload size:
FU_ALGOLTEK_AUX_FIRMWARE_PAYLOAD_SIZE =0x20000
Our new payload size:
FU_ALGOLTEK_AUX_FIRMWARE_PAYLOAD_SIZE =0x40000
(in firmware.h file)

When payload size is 0x20000 ,it will update success.
When payload size is 0x40000, it will update fail by using :
if (!fu_struct_algoltek_aux_isp_flash_write_cmd_address_pkt_set_data(
                                st,
                                fu_chunk_get_data(chk),
                                fu_chunk_get_data_sz(chk),
                                error)) {
                                g_prefix_error(error, "assign isp data failure: ");
                                return FALSE;
                     }

But it both update Success by using :
for (guint16 i= 0 ; i < 8; i++ ) {
                                st->data[i + 6] = firmwareBytesArray->data[i + firmwareBufferCount];
                                ...
}

This situation has been discovered before, so I didn't use chk in this function. I am not sure if it is caused by the payload being too large.
The Success and Fail log Already sent to you via e-mail.

@hughsie
Copy link
Member Author

hughsie commented Apr 29, 2024

Hi Jane, I've pushed a few more commits to the branch -- can you try again please?

@YiJaneWu
Copy link
Collaborator

Hi, actually FU_ALGOLTEK_AUX_FIRMWARE_PAYLOAD_SIZE needs to be set to 0x40000.
But it may fail due to the payload being too large. It’s just that 0x20000 won’t have this problem.

@hughsie
Copy link
Member Author

hughsie commented Apr 29, 2024

@YiJaneWu ahh, apologies -- I've reverted that patch -- can you grab me the logs with this branch please? There should be an actual error rather than a critical warning this time.

@YiJaneWu
Copy link
Collaborator

@hughsie Problem appear: failed to get at 0x20008 for 0x8: no data could be read.

@hughsie
Copy link
Member Author

hughsie commented Apr 30, 2024

failed to get at 0x20008 for 0x8

Can you email me the cab file please?

@hughsie hughsie force-pushed the algoltek/initial branch 2 times, most recently from 7668590 to 9b1b922 Compare May 1, 2024 18:56
@hughsie
Copy link
Member Author

hughsie commented May 2, 2024

@YiJaneWu can you get the new error message with this rebased upstream branch? Thanks.

@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 2, 2024

@hughsie The Email has been sent.
New error message is "did not find magic", I can't find my bin file.

@hughsie
Copy link
Member Author

hughsie commented May 2, 2024

New error message is "did not find magic", I can't find my bin file.

I've pushed a fix, can you try again please.

@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 2, 2024

Message: Base stream was 0x2104c bytes in size, and tried to create partial stream @0x104b of 0x40000 bytes.

@hughsie
Copy link
Member Author

hughsie commented May 2, 2024

Message: Base stream was 0x2104c bytes in size, and tried to create partial stream @0x104b of 0x40000 bytes.

Okay, that's actually a good error message now -- and what I was expecting. If we take a step back, the firmware.bin you sent me is 135244 bytes in size -- which is 0x2104C in hex.

We're laying out the firmware like this:

[FuStructAlgoltekAuxProductIdentity] @ 0x0 for 0x4B bytes
[ISP] @ 0x4B for 0x1000 bytes
[PAYLOAD] @ 0x104B for 0x40000

So the firmware.bin is too small -- no?

@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 3, 2024

Apologies, I found my problem here. I use another firmware and it can be update. But I need correct the payload size and use original bin file.
I don't have any problems here now. Thank you so much.

Signed-off-by: Richard Hughes <richard@hughsie.com>
@hughsie hughsie merged commit 51f59f0 into main May 3, 2024
18 checks passed
@hughsie hughsie deleted the algoltek/initial branch May 3, 2024 09:04
@hughsie
Copy link
Member Author

hughsie commented May 3, 2024

@YiJaneWu do you need this plugin in 1.9.x releases too? e.g. what goes into ChromeOS?

@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 3, 2024

Yes! I need this plugin in 1.9.x releases please.
I've learned a lot from you. Thank you so much!

@hughsie
Copy link
Member Author

hughsie commented May 3, 2024

@YiJaneWu #7202

@hughsie
Copy link
Member Author

hughsie commented May 3, 2024

@YiJaneWu on my system (with a Synaptics MST device!) I'm getting a console warning -- we probably ought to restrict getting the RDV down to just your devices :)

Using something like:

diff --git a/plugins/algoltek-aux/fu-algoltek-aux-plugin.c b/plugins/algoltek-aux/fu-algoltek-aux-plugin.c
index 5d163e48b..718ba2693 100644
--- a/plugins/algoltek-aux/fu-algoltek-aux-plugin.c
+++ b/plugins/algoltek-aux/fu-algoltek-aux-plugin.c
@@ -30,6 +30,7 @@ fu_algoltek_aux_plugin_backend_device_added(FuPlugin *plugin,
	/* interesting device? */
	if (!FU_IS_DPAUX_DEVICE(device))
		return TRUE;
+	g_warning("%s", fu_device_to_string(device));
 
	/* instantiate a new device */
	dev = fu_algoltek_aux_device_new(FU_DPAUX_DEVICE(device));

can you tell me what you get? e.g. on my system I get:

13:05:40.786 FuPluginAlgoltekAux  FuDpauxDevice:
  DeviceId:             2d9bd19a73af0e1160733d42c55f1d9de90ed451
  Name:                 AUX D/DDI D/PHY D
  Flags:                none
  Version:              0.5.1793
  VersionFormat:        triplet
  VersionRaw:           0x00050701
  PhysicalId:           PCI_SLOT_NAME=0000:00:02.0
  LogicalId:            drm_dp_aux3
  BackendId:            /sys/devices/pci0000:00/0000:00:02.0/drm/card1/card1-DP-3/drm_dp_aux3
  AcquiesceDelay:       2500
  PossiblePlugin:       algoltek_aux
  PossiblePlugin:       kinetic_dp
  PossiblePlugin:       mediatek_scaler
  PossiblePlugin:       synaptics_mst
  InternalFlags:        no-generic-guids
  Subsystem:            drm_dp_aux_dev
  DeviceFile:           /dev/drm_dp_aux3
  SysfsPath:            /sys/devices/pci0000:00/0000:00:02.0/drm/card1/card1-DP-3/drm_dp_aux3
  DpcdIeeeOui:          0x90cc24
  DpcdHwRev:            0x10
  DpcdDevId:            SYNAS2

I'm hoping we can filter your devices by DpcdIeeeOui or DpcdDevId.

@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 6, 2024

I got this:

01:48:50.851 FuMain ADDED:
FuAlgoltekAuxDevice:
DeviceId: d762543f8c20f636e6fff031a000078d3e10c600
Name: AUX B/DDI B/PHY B
Guid: 469a5c78-85ce-5086-9615-9347b9b21278 ← MST\VEN_25A4&DEV_AG9421
Plugin: algoltek_aux
Protocol: tw.com.algoltek.aux
Flags: updatable|registered|unsigned-payload
VendorId: DRM_DP_AUX_DEV:0x25A4
Version: AG9421_00.09.19_3.1.4_4.2.0-LVFS_AUX_v10-3
VersionFormat: plain
Created: 2024-05-06
PhysicalId: PCI_SLOT_NAME=0000:00:02.0
LogicalId: drm_dp_aux1
BackendId: /sys/devices/pci0000:00/0000:00:02.0/drm/card1/card1-DP-1/drm_dp_aux1
RemoveDelay: 10000
AcquiesceDelay: 2500
FirmwareGType: FuAlgoltekAuxFirmware
FirmwareGType: FuAlgoltekAuxFirmware
PossiblePlugin: algoltek_aux
PossiblePlugin: algoltek-aux
InternalFlags: is-open|only-wait-for-replug|no-generic-guids
Subsystem: drm_dp_aux_dev
DeviceFile: /dev/drm_dp_aux1
SysfsPath: /sys/devices/pci0000:00/0000:00:02.0/drm/card1/card1-DP-1/drm_dp_aux1

and I also got the red warning :
failed to add device /sys/devices/pci0000:00/0000:00:02.0/drm/card1/card1-eDP-1/drm_dp_aux0: failed to add device using on algoltek_aux: aux dpcd write failed: failed to write 6 bytes to 18: Input/output error

@hughsie
Copy link
Member Author

hughsie commented May 6, 2024

01:48:50.851 FuMain ADDED: FuAlgoltekAuxDevice:

Did you get any others? I was expecting to see some Dpcd properties there too. The whole log would be useful.

@YiJaneWu YiJaneWu restored the algoltek/initial branch May 7, 2024 03:47
@YiJaneWu YiJaneWu deleted the algoltek/initial branch May 7, 2024 03:49
@YiJaneWu
Copy link
Collaborator

YiJaneWu commented May 7, 2024

Hi, the log is too long and the email has been sent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants