Xilinx / dfx-mgr

Other
11 stars 6 forks source link

add FIRMWARES_DIRECTORY configuration option to dfx-mgr cli args #7

Closed talhaHavadar closed 4 weeks ago

talhaHavadar commented 4 months ago

Added a new argument into dfx-mgr cli to enable users to use custom firmware directory to search for bitstreams

Also uncomment disabled install as result of patch from debian package

rajimoha commented 2 months ago

@talhaHavadar

please use the following coding style https://www.kernel.org/doc/html/v4.10/process/coding-style.html

Section "3) Placing Braces and Spaces"

rajimoha commented 2 months ago

I dont think we need to pass firmware location as argument since it is provided as configurable in below file "/etc/dfx-mgrd/daemon.conf"

content of file: $ cat /etc/dfx-mgrd/daemon.conf { "default_accel":"/etc/dfx-mgrd/default_firmware", //Optional: echo the //package-name to default_firmware for any // accelerator to be loaded on start of daemon "firmware_location": ["/lib/firmware/xilinx"] //Required:Package directories // monitored by DFX-MGR } firmware_location -- this can be updated as per the documentation provided for daemon.conf

One more aspect which we need to look at is the different types of PL design: 1) For PL_FLAT type the directory can just be in the firmware location 2) For PL_DFX type a specific structure needs to be following: firmware_location///_slot# ( _slot0 for slot 0, _slot1 for slot 1 etc)

arunkann commented 2 months ago

Hi Talha Havadar, I am Arun Kannan from AMD. I work with Wes Skeffington. Can you please clarify why do you want to change the location of the daemon config file ? What's the context in Ubuntu that the current location of config file doesn't work ? We want to understand the problem better.

The current config file does allow to pick up firmwares from different directories. You just need to update the config file & relaunch the daemon.

Please get back to our query.

talhaHavadar commented 4 weeks ago

Hello all thank you very much for the feedback and guidance, I will update the mechanism pretty drastically so I think it is best to close this PR for now.