Re: [PATCH drm-next v2] drm/hyperv: Replace simple-KMS with regular atomic helpers

[Javier Martinez Canillas <javierm@redhat.com>]

Ryosuke Yasuoka <ryasuoka@redhat.com> writes:

Hello Ryosuke,

> Hi Javier,
>
> On Fri, Apr 25, 2025 at 4:15 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>>
>> Ryosuke Yasuoka <ryasuoka@redhat.com> writes:
>>
>> Hello Ryosuke,
>>
>> > Drop simple-KMS in favor of regular atomic helpers to make the code more
>> > modular. The simple-KMS helper mix up plane and CRTC state, so it is
>> > obsolete and should go away [1]. Since it just split the simple-pipe
>> > functions into per-plane and per-CRTC, no functional changes is
>> > expected.
>> >
>> > [1] https://lore.kernel.org/lkml/dae5089d-e214-4518-b927-5c4149babad8@suse.de/
>> >
>> > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>> >
>>
>>
>>
>> > -static void hyperv_pipe_enable(struct drm_simple_display_pipe *pipe,
>> > -                            struct drm_crtc_state *crtc_state,
>> > -                            struct drm_plane_state *plane_state)
>> > +static const uint32_t hyperv_formats[] = {
>> > +     DRM_FORMAT_XRGB8888,
>> > +};
>> > +
>> > +static const uint64_t hyperv_modifiers[] = {
>> > +     DRM_FORMAT_MOD_LINEAR,
>> > +     DRM_FORMAT_MOD_INVALID
>> > +};
>> > +
>>
>> I think the kernel u32 and u64 types are preferred ?
>
> I'm not sure if I should fix this in this patch because I did not add these
> variables. IMO, we need to split the commit if we fix them.
>

Right, I got confused for how the diff showed the changes. But I agree with
you that should be a separate patch since the variables already exist.

[...]

>>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Thank you for your review and comment. I'll fix them and add your ack.
>

Thanks!

> Best regards,
> Ryosuke
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat