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

Fix build issues with Linux v6.8 and later #39

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

hungyuhang
Copy link
Contributor

  1. Rename min_buffers_needed member in struct vb2_queue to min_queued_buffers according to the changes in Linux v6.8.
    (The change described above in the Linux Kernel source code can be viewed at: torvalds/linux@80c2b40)

  2. Remove FBINFO_FLAG_DEFAULT flag, as this flag was removed in Linux v6.6 and later.
    (The change described above in the Linux Kernel source code can be viewed at: torvalds/linux@0444fa3)

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Improve the git commit messages.

Read https://cbea.ms/git-commit/ carefully and refine.

fb.c Outdated
@@ -402,7 +402,7 @@ int vcamfb_init(struct vcam_device *dev)
info->fbops = &vcamfb_ops;
info->par = dev;
info->pseudo_palette = NULL;
info->flags = FBINFO_FLAG_DEFAULT;
info->flags = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you set the flags properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below is a commit of the Linux DRM subsystem which deals with the removal of the FBINFO_FLAG_DEFAULT macro:
torvalds/linux@40e324e

According to the commit above, the default value of fb_info.flags will be set properly using framebuffer_alloc(), which is zero in the current Linux version. So I assumed that fb_info.flags can be set to zero at initialization.

But for better portability, I think I should use framebuffer_alloc() to initialize the value of fb_info.flags instead of explicitlly setting it to zero. I will modify the code and commit it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good. Always address the rationale in comments or git commit messages.

@jserv
Copy link
Collaborator

jserv commented Jun 27, 2024

It is meaningless to say something like

In Linux v6.8, the 'min_buffers_needed' field of vb2_queue was renamed
to 'min_queued_buffers'. Due to this change, the compiler will not find
the declaration of the field 'min_buffers_needed' and will cause
compilation error when building with Linux v6.8 and later.

The compiler is not related to the changes by Linux kernel internally. You should analyze the upstream changes for the considerations and/or impacts.

The field 'min_buffers_needed' in vb2_queue was renamed to
'min_queued_buffers' in Linux kernel v6.8. As a result, the field name
needs to be updated when building the driver on newer kernel versions.

For backward compatibility, #if and #else macros have been added to
select the correct field name based on the kernel version.
The reason to do such changes is that the flag FBINFO_DEFAULT has
been removed from the Linux kernel since v6.6, which was the default
value of fb_info.flags.

To initialize the fb_info structure (and the value of fb_info.flags)
correctly without using FBINFO_DEFAULT, the framebuffer_alloc()
function provided by the Linux kernel should be used.

Since framebuffer_alloc() only accepts pointer to fb_info, the type of
the info field in struct vcamfb_info was changed from
'struct fb_info' to 'pointer to struct fb_info'.

As a result, framebuffer_release() was added to deallocate the memory
pointed to by the info field. Additionally, several changes were made
to accommodate the change of the info field's type.
@hungyuhang
Copy link
Contributor Author

It is meaningless to say something like

In Linux v6.8, the 'min_buffers_needed' field of vb2_queue was renamed
to 'min_queued_buffers'. Due to this change, the compiler will not find
the declaration of the field 'min_buffers_needed' and will cause
compilation error when building with Linux v6.8 and later.

The compiler is not related to the changes by Linux kernel internally. You should analyze the upstream changes for the considerations and/or impacts.

I've updated the commit messages.

@jserv jserv merged commit 7156b86 into sysprog21:master Jul 15, 2024
2 checks passed
@jserv
Copy link
Collaborator

jserv commented Jul 15, 2024

Thank @hungyuhang for contributing!

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

Successfully merging this pull request may close these issues.

2 participants