[PATCH] Introduce Tyr
Maíra Canal
mcanal at igalia.com
Thu Jul 3 10:45:56 UTC 2025
Hi Daniel,
On 30/06/25 10:53, Daniel Almeida wrote:
> Hi Maíra, thanks for chiming in :)
>
>>
>> To enhance readability, consider using a regmap similar to
>> panthor_regs.h. This would help avoid 'magic numbers' and make the
>> code's intent much clearer.
>
>
> Are you referring to "struct regmap" itself? Because last I checked, this
> abstraction is not available upstream. There was a person working on it, but I
> guess it hasn't seen any traction for a few months. I also don't see it being
> used in panthor_regs.h?
Sorry, I think I didn't express myself clearly. When I say regmap, I
mean using macros to express the register addresses and its fields. From
example, in Panthor, "1 | bit_u32(8)" is expressed as
GPU_IRQ_RESET_COMPLETED, which can make things more readable.
Best Regards,
- Maíra
>
>>
>>> + regs::GPU_CMD.write(iomem, irq_enable_cmd)?;
>>> +
>>> + let op = || regs::GPU_INT_RAWSTAT.read(iomem);
>>> + let cond = |raw_stat: &u32| -> bool { (*raw_stat >> 8) & 1 == 1 };
>>> + let res = io::poll::read_poll_timeout(
>>> + op,
>>> + cond,
>>> + time::Delta::from_millis(100),
>>> + Some(time::Delta::from_micros(20000)),
>>> + );
>>> +
>>> + if let Err(e) = res {
>>> + pr_err!("GPU reset failed with errno {}\n", e.to_errno());
>>> + pr_err!(
>>> + "GPU_INT_RAWSTAT is {}\n",
>>> + regs::GPU_INT_RAWSTAT.read(iomem)?
>>> + );
>>> + }
>>> +
>>> + Ok(())
>>> +}
>>> +
>>> +kernel::of_device_table!(
>>> + OF_TABLE,
>>> + MODULE_OF_TABLE,
>>> + <TyrDriver as platform::Driver>::IdInfo,
>>> + [
>>> + (of::DeviceId::new(c_str!("rockchip,rk3588-mali")), ()),
>>> + (of::DeviceId::new(c_str!("arm,mali-valhall-csf")), ())
>>> + ]
>>> +);
>>> +
>>> +impl platform::Driver for TyrDriver {
>>> + type IdInfo = ();
>>> + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
>>> +
>>> + fn probe(
>>> + pdev: &platform::Device<Core>,
>>> + _info: Option<&Self::IdInfo>,
>>> + ) -> Result<Pin<KBox<Self>>> {
>>> + dev_dbg!(pdev.as_ref(), "Probed Tyr\n");
>>> +
>>> + let core_clk = Clk::get(pdev.as_ref(), Some(c_str!("core")))?;
>>> + let stacks_clk = Clk::get(pdev.as_ref(), Some(c_str!("stacks")))?;
>>
>> Shouldn't it be OptionalClk::get? From the DT schema for "arm,mali-
>> valhall-csf", I see that "stacks" and "coregroups" are optional.
>>
>>> + let coregroup_clk = Clk::get(pdev.as_ref(), Some(c_str!("coregroup")))?;
>>
>> Same.
>>
>> Best Regards,
>> - Maíra
>>
>>
>
> Ah yes, you’re right. I will fix that in v2.
>
> — Daniel
>
More information about the dri-devel
mailing list