Re: [BUG] gpu: drm: radeon: two possible deadlocks involving locking and waiting
Hi Jia-Ju, interesting that you have found those issues with an automated tool. And yes that is a well design flaw within the radeon driver which can happen on hardware faults, e.g. when radeon_ring_backup() needs to be called. But that happens so rarely and the driver is not developed further that we decided to not address this any more. Regards, Christian. Am 01.02.22 um 08:40 schrieb Jia-Ju Bai: Hello, My static analysis tool reports a possible deadlock in the radeon driver in Linux 5.16: #BUG 1 radeon_dpm_change_power_state_locked() mutex_lock(&rdev->ring_lock); --> Line 1133 (Lock A) radeon_fence_wait_empty() radeon_fence_wait_seq_timeout() wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X) radeon_ring_backup() mutex_lock(&rdev->ring_lock); --> Line 289(Lock A) radeon_fence_count_emitted() radeon_fence_process() wake_up_all(&rdev->fence_queue); --> Line 323 (Wake X) When radeon_dpm_change_power_state_locked() is executed, "Wait X" is performed by holding "Lock A". If radeon_ring_backup() is executed at this time, "Wake X" cannot be performed to wake up "Wait X" in radeon_dpm_change_power_state_locked(), because "Lock A" has been already hold by radeon_dpm_change_power_state_locked(), causing a possible deadlock. I find that "Wait X" is performed with a timeout MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think this timeout can cause inefficient execution. #BUG 2 radeon_ring_lock() mutex_lock(&rdev->ring_lock); --> Line 147 (Lock A) radeon_ring_alloc() radeon_fence_wait_next() radeon_fence_wait_seq_timeout() wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X) radeon_ring_backup() mutex_lock(&rdev->ring_lock); --> Line 289(Lock A) radeon_fence_count_emitted() radeon_fence_process() wake_up_all(&rdev->fence_queue); --> Line 323 (Wake X) When radeon_ring_lock() is executed, "Wait X" is performed by holding "Lock A". If radeon_ring_backup() is executed at this time, "Wake X" cannot be performed to wake up "Wait X" in radeon_ring_lock(), because "Lock A" has been already hold by radeon_ring_lock(), causing a possible deadlock. I find that "Wait X" is performed with a timeout MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think this timeout can cause inefficient execution. I am not quite sure whether these possible problems are real and how to fix them if they are real. Any feedback would be appreciated, thanks :) Best wishes, Jia-Ju Bai
Re: [PATCH 00/14] Rename dma-buf-map
Am 01.02.22 um 01:36 schrieb Lucas De Marchi: On Fri, Jan 28, 2022 at 10:48:42AM +0100, Christian König wrote: Am 28.01.22 um 10:40 schrieb Lucas De Marchi: On Fri, Jan 28, 2022 at 10:22:00AM +0100, Christian König wrote: Am 28.01.22 um 10:12 schrieb Lucas De Marchi: On Fri, Jan 28, 2022 at 09:41:14AM +0100, Christian König wrote: Rule #1 is to never ever break the build. Because of this all those patches needs to be squashed into a single one as far as I can see. what config are you building on? Well I'm not building at all, I'm just looking at the patches as an engineer with 25 years of experience with Linux patches. Just take a look at patch number 2: -static int fastrpc_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map) +static int fastrpc_vmap(struct dma_buf *dmabuf, struct iosys_map *map) You are changing the functions signature without changing any of the callers. At bare minimum that causes a warning and on runtime this only works by coincident now because the structure pointers just happen to have the same layout. This is not something we usually do. you missed the magic/hack on patch 1: 1) dma-buf-map.h includes iosys-map.h _at the end_ 2) iosys-map.h includes dma-buf-map.h at the beginning and initially does a "define iosys_map dma_buf_map". So, it doesn't work by coincidence, It's because it was done to allow converting it piecemeal. Oh, my. Please never do stuff like that again. It's not uncommon approach to be required by other subsystems. Even drm-intel already used similar approach for macro conversions crossing drm-intel-next and drm-intel-gt-next branches recently. As I said, I don't mind one way or the other. The key point is that you seemed to have a misunderstanding why we separate changes into functional independent patches. The goal of that is *not* to reduce the number of lines in a patch, but rather to reduce the complexity of the review. When you do an automated renamed with a cocci or sed script you can have a 100k line patch as result, which is perfectly fine to send out like this as long as you include the script/commands used to autogenerate the patch. The background is that everybody on the planet can generate the patch with those commands himself and see if the results matches your patch or not. The maintainer of the component can then just puts an Acked-by on the patch and move on, but separating the patch causes additional work for both you as well as the reviewers. Separating the change into individual patches as much as possible is nice to have when you do a functional change and want or need a review from each individual driver maintainer. This is usually the default case, so sticking with separated changes as much as possible is usually still the best practice. Before I go and respin this into a single mega patch, I'd like to gather some feedback on the following topics: 1) Daniel Vetter and Thomas Zimmermann seemed to be ok with staying with the current name, dma_buf_map, while you prefer it renamed. Or at least not make the rename a pre-requisite for the API additions in https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220126203702.1784589-1-lucas.demarchi%40intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C01142fa3ce484040ade008d9e51aef5d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637792726123940514%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ieMZ9Jiwo%2FQpT5kyyQgHNlepiusN%2Fkfff1Op6TVQ%2BaA%3D&reserved=0 One thing I like about the rename is that it makes clear the separation between this small shim and dma-buf. There are also some APIs that are really dma-buf API (e.g. dma_buf_map_attachment()), but if you don't look carefully you may think it's from dma_buf_map. Exactly that's the reason why I see this rename as mandatory. Adding the functionality goes beyond the inter driver interface DMA-buf provides into driver internal territory and I want to make sure that people understand just from the name alone that this is not part of DMA-buf but rather an independent functionality. 2) If renaming, would it still keep the same entry in MAINTAINERS? Thomas suggested drivers core, but this all seem to be used mainly on drm/, with just one exception. I would just add a complete new entry for this and use Thomas as maintainer (with his permission of course) and dri as mailing list. 3) If renaming, do we have another preferred name? Nope, as Daniel said the name itself is only bikesheed. What is important is that we see this as functionality separated from the inter driver interface. Regards, Christian. thanks Lucas De Marchi But as I said, I don't really have a preference. When crossing subsystems one thing that is hard is that different people have different preferences on these things. At least squashing now is much easier than if I had to split it Try to i
[BUG] gpu: drm: radeon: two possible deadlocks involving locking and waiting
Hello, My static analysis tool reports a possible deadlock in the radeon driver in Linux 5.16: #BUG 1 radeon_dpm_change_power_state_locked() mutex_lock(&rdev->ring_lock); --> Line 1133 (Lock A) radeon_fence_wait_empty() radeon_fence_wait_seq_timeout() wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X) radeon_ring_backup() mutex_lock(&rdev->ring_lock); --> Line 289(Lock A) radeon_fence_count_emitted() radeon_fence_process() wake_up_all(&rdev->fence_queue); --> Line 323 (Wake X) When radeon_dpm_change_power_state_locked() is executed, "Wait X" is performed by holding "Lock A". If radeon_ring_backup() is executed at this time, "Wake X" cannot be performed to wake up "Wait X" in radeon_dpm_change_power_state_locked(), because "Lock A" has been already hold by radeon_dpm_change_power_state_locked(), causing a possible deadlock. I find that "Wait X" is performed with a timeout MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think this timeout can cause inefficient execution. #BUG 2 radeon_ring_lock() mutex_lock(&rdev->ring_lock); --> Line 147 (Lock A) radeon_ring_alloc() radeon_fence_wait_next() radeon_fence_wait_seq_timeout() wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X) radeon_ring_backup() mutex_lock(&rdev->ring_lock); --> Line 289(Lock A) radeon_fence_count_emitted() radeon_fence_process() wake_up_all(&rdev->fence_queue); --> Line 323 (Wake X) When radeon_ring_lock() is executed, "Wait X" is performed by holding "Lock A". If radeon_ring_backup() is executed at this time, "Wake X" cannot be performed to wake up "Wait X" in radeon_ring_lock(), because "Lock A" has been already hold by radeon_ring_lock(), causing a possible deadlock. I find that "Wait X" is performed with a timeout MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think this timeout can cause inefficient execution. I am not quite sure whether these possible problems are real and how to fix them if they are real. Any feedback would be appreciated, thanks :) Best wishes, Jia-Ju Bai
[PATCH v2 4/4] drm/i915/display: Simplify helpers for getting DSC slices and bpp
Genralize the helper for getting DSC slice count and compressed bpp for HDMI sink supporting DSC. This patch: -Removes the assumption on the bpc and sends it as an argument for calculating compressed bpc. -Sends the resolution, and output format as parameters for which the DSC paremeters are to be calculated instead of crtc_state. v2: Added forward declaration for struct drm_display_mode. Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/i915/display/intel_dp.c | 7 +-- drivers/gpu/drm/i915/display/intel_hdmi.c | 24 --- drivers/gpu/drm/i915/display/intel_hdmi.h | 6 -- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index f7fe7de7e553..17d08f06499b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2357,7 +2357,9 @@ intel_dp_pcon_dsc_enc_slices(struct intel_dp *intel_dp, int pcon_max_slices = drm_dp_pcon_dsc_max_slices(intel_dp->pcon_dsc_dpcd); int pcon_max_slice_width = drm_dp_pcon_dsc_max_slice_width(intel_dp->pcon_dsc_dpcd); - return intel_hdmi_dsc_get_num_slices(crtc_state, pcon_max_slices, + return intel_hdmi_dsc_get_num_slices(&crtc_state->hw.adjusted_mode, +crtc_state->output_format, +pcon_max_slices, pcon_max_slice_width, hdmi_max_slices, hdmi_throughput); } @@ -2374,9 +2376,10 @@ intel_dp_pcon_dsc_enc_bpp(struct intel_dp *intel_dp, int pcon_fractional_bpp = drm_dp_pcon_dsc_bpp_incr(intel_dp->pcon_dsc_dpcd); int hdmi_max_chunk_bytes = connector->display_info.hdmi.dsc_cap.total_chunk_kbytes * 1024; + int bpc = crtc_state->pipe_bpp / 3; return intel_hdmi_dsc_get_bpp(pcon_fractional_bpp, slice_width, - num_slices, output_format, hdmi_all_bpp, + num_slices, output_format, bpc, hdmi_all_bpp, hdmi_max_chunk_bytes); } diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 381a9de3a015..f75e2384da63 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -3004,7 +3004,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive) * intel_hdmi_dsc_get_num_slices - get no. of dsc slices based on dsc encoder * and dsc decoder capabilities * - * @crtc_state: intel crtc_state + * @mode: drm_display_mode for which num of slices are needed + * @output_format : pipe output format * @src_max_slices: maximum slices supported by the DSC encoder * @src_max_slice_width: maximum slice width supported by DSC encoder * @hdmi_max_slices: maximum slices supported by sink DSC decoder @@ -3014,7 +3015,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive) * and decoder. */ int -intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, +intel_hdmi_dsc_get_num_slices(const struct drm_display_mode *mode, + enum intel_output_format output_format, int src_max_slices, int src_max_slice_width, int hdmi_max_slices, int hdmi_throughput) { @@ -3036,7 +3038,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, int max_throughput; /* max clock freq. in khz per slice */ int max_slice_width; int slice_width; - int pixel_clock = crtc_state->hw.adjusted_mode.crtc_clock; + int pixel_clock = mode->crtc_clock; if (!hdmi_throughput) return 0; @@ -3047,8 +3049,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, * for 4:4:4 is 1.0. Multiplying these factors by 10 and later * dividing adjusted clock value by 10. */ - if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 || - crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) + if (output_format == INTEL_OUTPUT_FORMAT_YCBCR444 || + output_format == INTEL_OUTPUT_FORMAT_RGB) kslice_adjust = 10; else kslice_adjust = 5; @@ -3103,7 +3105,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, else return 0; - slice_width = DIV_ROUND_UP(crtc_state->hw.adjusted_mode.hdisplay, target_slices); + slice_width = DIV_ROUND_UP(mode->hdisplay, target_slices); if (slice_width >= max_slice_width) min_slices = target_slices + 1; } while (slice_width >= max_slice_width); @@ -3119,6 +3121,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, * @slice_width: dsc slice width
[PATCH v2 0/4] Minor Fixes and Refactoring for HDMI PCON stuff
Misc fixes and refactoring in HDMI2.1 PCON helper functions. V2: Addressed review comments from Jani. Splitted the drm_helper addition and usage in separate patches. Ankit Nautiyal (4): drm/i915/hdmi: Fix the definition of intel_hdmi_dsc_get_bpp drm/edid: Add helper to get max FRL rate for an HDMI sink drm/i915/dp: Use the drm helpers for getting max FRL rate drm/i915/display: Simplify helpers for getting DSC slices and bpp drivers/gpu/drm/drm_edid.c| 38 +++ drivers/gpu/drm/i915/display/intel_dp.c | 26 ++-- drivers/gpu/drm/i915/display/intel_hdmi.c | 26 +--- drivers/gpu/drm/i915/display/intel_hdmi.h | 9 -- include/drm/drm_edid.h| 2 ++ 5 files changed, 70 insertions(+), 31 deletions(-) -- 2.25.1
[PATCH v2 3/4] drm/i915/dp: Use the drm helpers for getting max FRL rate
Re-use the drm helpers for getting max FRL rate for an HDMI sink. This patch removes the duplicate code and calls the already defined drm helpers for the task. Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/i915/display/intel_dp.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 4d4579a301f6..f7fe7de7e553 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2190,22 +2190,13 @@ static int intel_dp_hdmi_sink_max_frl(struct intel_dp *intel_dp) { struct intel_connector *intel_connector = intel_dp->attached_connector; struct drm_connector *connector = &intel_connector->base; - int max_frl_rate; - int max_lanes, rate_per_lane; - int max_dsc_lanes, dsc_rate_per_lane; + int max_frl = drm_hdmi_sink_max_frl(connector); + int dsc_max_frl = drm_hdmi_sink_dsc_max_frl(connector); - max_lanes = connector->display_info.hdmi.max_lanes; - rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane; - max_frl_rate = max_lanes * rate_per_lane; + if (dsc_max_frl) + return min(max_frl, dsc_max_frl); - if (connector->display_info.hdmi.dsc_cap.v_1p2) { - max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes; - dsc_rate_per_lane = connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane; - if (max_dsc_lanes && dsc_rate_per_lane) - max_frl_rate = min(max_frl_rate, max_dsc_lanes * dsc_rate_per_lane); - } - - return max_frl_rate; + return max_frl; } static bool -- 2.25.1
[PATCH v2 1/4] drm/i915/hdmi: Fix the definition of intel_hdmi_dsc_get_bpp
Fix the data-type of the argument output_format to enum, for the function intel_hdmi_dsc_get_bpp. v2: Fixed formatting issues in commit message (Jani). Avoided including the header intel_display_types.h, instead used forward declaration for the appropriate enum. (Jani). Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC parameters for HDMI2.1") Cc: Uma Shankar Cc: Jani Nikula Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++-- drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 45cf0ab04009..381a9de3a015 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, */ int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices, - int output_format, bool hdmi_all_bpp, - int hdmi_max_chunk_bytes) + enum intel_output_format output_format, + bool hdmi_all_bpp, int hdmi_max_chunk_bytes) { int max_dsc_bpp, min_dsc_bpp; int target_bytes; diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h index b577c38fa90c..ea2a3456bd4b 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.h +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h @@ -22,6 +22,7 @@ struct intel_hdmi; struct drm_connector_state; union hdmi_infoframe; enum port; +enum intel_output_format; void intel_hdmi_init_connector(struct intel_digital_port *dig_port, struct intel_connector *intel_connector); @@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state, bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state, int bpc, bool has_hdmi_sink, bool ycbcr420_output); int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, - int num_slices, int output_format, bool hdmi_all_bpp, - int hdmi_max_chunk_bytes); + int num_slices, enum intel_output_format output_format, + bool hdmi_all_bpp, int hdmi_max_chunk_bytes); int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state, int src_max_slices, int src_max_slice_width, int hdmi_max_slices, int hdmi_throughput); -- 2.25.1
[PATCH v2 2/4] drm/edid: Add helper to get max FRL rate for an HDMI sink
Add the helpers for getting the max FRL rate with and without DSC for an HDMI sink. v2: Fix the subject line and documentation of the helpers (Jani). Split the helper definitions and usage into separate patches. (Jani). Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/drm_edid.c | 38 ++ include/drm/drm_edid.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index eb61a1a92dc0..c209fd6b24a2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6176,3 +6176,41 @@ void drm_update_tile_info(struct drm_connector *connector, connector->tile_group = NULL; } } + +/** + * drm_hdmi_sink_max_frl - get the max frl rate, if supported + * @connector - connector with HDMI sink + * + * RETURNS: + * max frl rate supported by the HDMI sink, 0 if FRL not supported + */ +int drm_hdmi_sink_max_frl(struct drm_connector *connector) +{ + int max_lanes = connector->display_info.hdmi.max_lanes; + int rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane; + + return max_lanes * rate_per_lane; +} +EXPORT_SYMBOL(drm_hdmi_sink_max_frl); + +/** + * drm_hdmi_sink_dsc_max_frl - get the max frl rate from HDMI sink with + * DSC1.2 compression. + * @connector - connector with HDMI sink + * + * RETURNS: + * max frl rate supported by the HDMI sink with DSC1.2, 0 if FRL not supported + */ +int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector) +{ + int max_dsc_lanes, dsc_rate_per_lane; + + if (!connector->display_info.hdmi.dsc_cap.v_1p2) + return 0; + + max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes; + dsc_rate_per_lane = connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane; + + return max_dsc_lanes * dsc_rate_per_lane; +} +EXPORT_SYMBOL(drm_hdmi_sink_dsc_max_frl); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 18f6c700f6d0..5003e1254c44 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -592,6 +592,8 @@ drm_display_mode_from_cea_vic(struct drm_device *dev, u8 video_code); const u8 *drm_find_edid_extension(const struct edid *edid, int ext_id, int *ext_index); +int drm_hdmi_sink_max_frl(struct drm_connector *connector); +int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector); #endif /* __DRM_EDID_H__ */ -- 2.25.1
[PATCH 3/3] tools: add hmm gup test for long term pinned device pages
From: Alex Sierra The intention is to test device coherent type pages that have been called through get user pages with PIN_LONGTERM flag set. These pages should get migrated back to normal system memory. Signed-off-by: Alex Sierra Signed-off-by: Alistair Popple --- tools/testing/selftests/vm/Makefile| 2 +- tools/testing/selftests/vm/hmm-tests.c | 81 +++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile index 1607322..58c8427 100644 --- a/tools/testing/selftests/vm/Makefile +++ b/tools/testing/selftests/vm/Makefile @@ -142,7 +142,7 @@ $(OUTPUT)/mlock-random-test $(OUTPUT)/memfd_secret: LDLIBS += -lcap $(OUTPUT)/gup_test: ../../../../mm/gup_test.h -$(OUTPUT)/hmm-tests: local_config.h +$(OUTPUT)/hmm-tests: local_config.h ../../../../mm/gup_test.h # HMM_EXTRA_LIBS may get set in local_config.mk, or it may be left empty. $(OUTPUT)/hmm-tests: LDLIBS += $(HMM_EXTRA_LIBS) diff --git a/tools/testing/selftests/vm/hmm-tests.c b/tools/testing/selftests/vm/hmm-tests.c index 84ec8c4..11b83a8 100644 --- a/tools/testing/selftests/vm/hmm-tests.c +++ b/tools/testing/selftests/vm/hmm-tests.c @@ -36,6 +36,7 @@ * in the usual include/uapi/... directory. */ #include "../../../../lib/test_hmm_uapi.h" +#include "../../../../mm/gup_test.h" struct hmm_buffer { void*ptr; @@ -60,6 +61,8 @@ enum { #define NTIMES 10 #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1))) +/* Just the flags we need, copied from mm.h: */ +#define FOLL_WRITE 0x01/* check pte is writable */ FIXTURE(hmm) { @@ -1766,4 +1769,82 @@ TEST_F(hmm, exclusive_cow) hmm_buffer_free(buffer); } +/* + * Test get user device pages through gup_test. Setting PIN_LONGTERM flag. + * This should trigger a migration back to system memory for both, private + * and coherent type pages. + * This test makes use of gup_test module. Make sure GUP_TEST_CONFIG is added + * to your configuration before you run it. + */ +TEST_F(hmm, hmm_gup_test) +{ + struct hmm_buffer *buffer; + struct gup_test gup; + int gup_fd; + unsigned long npages; + unsigned long size; + unsigned long i; + int *ptr; + int ret; + unsigned char *m; + + gup_fd = open("/sys/kernel/debug/gup_test", O_RDWR); + if (gup_fd == -1) + SKIP(return, "Skipping test, could not find gup_test driver"); + + npages = 4; + ASSERT_NE(npages, 0); + size = npages << self->page_shift; + + buffer = malloc(sizeof(*buffer)); + ASSERT_NE(buffer, NULL); + + buffer->fd = -1; + buffer->size = size; + buffer->mirror = malloc(size); + ASSERT_NE(buffer->mirror, NULL); + + buffer->ptr = mmap(NULL, size, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + buffer->fd, 0); + ASSERT_NE(buffer->ptr, MAP_FAILED); + + /* Initialize buffer in system memory. */ + for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i) + ptr[i] = i; + + /* Migrate memory to device. */ + ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages); + ASSERT_EQ(ret, 0); + ASSERT_EQ(buffer->cpages, npages); + /* Check what the device read. */ + for (i = 0, ptr = buffer->mirror; i < size / sizeof(*ptr); ++i) + ASSERT_EQ(ptr[i], i); + + gup.nr_pages_per_call = npages; + gup.addr = (unsigned long)buffer->ptr; + gup.gup_flags = FOLL_WRITE; + gup.size = size; + /* +* Calling gup_test ioctl. It will try to PIN_LONGTERM these device pages +* causing a migration back to system memory for both, private and coherent +* type pages. +*/ + if (ioctl(gup_fd, PIN_LONGTERM_BENCHMARK, &gup)) { + perror("ioctl on PIN_LONGTERM_BENCHMARK\n"); + goto out_test; + } + + /* Take snapshot to make sure pages have been migrated to sys memory */ + ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_SNAPSHOT, buffer, npages); + ASSERT_EQ(ret, 0); + ASSERT_EQ(buffer->cpages, npages); + m = buffer->mirror; + for (i = 0; i < npages; i++) + ASSERT_EQ(m[i], HMM_DMIRROR_PROT_WRITE); +out_test: + close(gup_fd); + hmm_buffer_free(buffer); +} TEST_HARNESS_MAIN -- git-series 0.9.1
[PATCH 2/3] mm/gup.c: Migrate device coherent pages when pinning instead of failing
Currently any attempts to pin a device coherent page will fail. This is because device coherent pages need to be managed by a device driver, and pinning them would prevent a driver from migrating them off the device. However this is no reason to fail pinning of these pages. These are coherent and accessible from the CPU so can be migrated just like pinning ZONE_MOVABLE pages. So instead of failing all attempts to pin them first try migrating them out of ZONE_DEVICE. Signed-off-by: Alistair Popple --- mm/gup.c | 105 ++-- 1 file changed, 95 insertions(+), 10 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index f596b93..2cbef54 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1834,6 +1834,60 @@ struct page *get_dump_page(unsigned long addr) #ifdef CONFIG_MIGRATION /* + * Migrates a device coherent page back to normal memory. Caller should have a + * reference on page which will be copied to the new page if migration is + * successful or dropped on failure. + */ +static struct page *migrate_device_page(struct page *page, + unsigned int gup_flags) +{ + struct page *dpage; + struct migrate_vma args; + unsigned long src_pfn, dst_pfn = 0; + + lock_page(page); + src_pfn = migrate_pfn(page_to_pfn(page)) | MIGRATE_PFN_MIGRATE; + args.src = &src_pfn; + args.dst = &dst_pfn; + args.cpages = 1; + args.npages = 1; + args.vma = NULL; + migrate_vma_setup(&args); + if (!(src_pfn & MIGRATE_PFN_MIGRATE)) + return NULL; + + dpage = alloc_pages(GFP_USER | __GFP_NOWARN, 0); + + /* +* get/pin the new page now so we don't have to retry gup after +* migrating. We already have a reference so this should never fail. +*/ + if (WARN_ON_ONCE(!try_grab_page(dpage, gup_flags))) { + __free_pages(dpage, 0); + dpage = NULL; + } + + if (dpage) { + lock_page(dpage); + dst_pfn = migrate_pfn(page_to_pfn(dpage)); + } + + migrate_vma_pages(&args); + if (src_pfn & MIGRATE_PFN_MIGRATE) + copy_highpage(dpage, page); + migrate_vma_finalize(&args); + if (dpage && !(src_pfn & MIGRATE_PFN_MIGRATE)) { + if (gup_flags & FOLL_PIN) + unpin_user_page(dpage); + else + put_page(dpage); + dpage = NULL; + } + + return dpage; +} + +/* * Check whether all pages are pinnable, if so return number of pages. If some * pages are not pinnable, migrate them, and unpin all pages. Return zero if * pages were migrated, or if some pages were not successfully isolated. @@ -1861,15 +1915,40 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages, continue; prev_head = head; /* -* If we get a movable page, since we are going to be pinning -* these entries, try to move them out if possible. +* Device coherent pages are managed by a driver and should not +* be pinned indefinitely as it prevents the driver moving the +* page. So when trying to pin with FOLL_LONGTERM instead try +* migrating page out of device memory. */ if (is_dev_private_or_coherent_page(head)) { + /* +* device private pages will get faulted in during gup +* so it shouldn't be possible to see one here. +*/ WARN_ON_ONCE(is_device_private_page(head)); - ret = -EFAULT; - goto unpin_pages; + WARN_ON_ONCE(PageCompound(head)); + + /* +* migration will fail if the page is pinned, so convert +* the pin on the source page to a normal reference. +*/ + if (gup_flags & FOLL_PIN) { + get_page(head); + unpin_user_page(head); + } + + pages[i] = migrate_device_page(head, gup_flags); + if (!pages[i]) { + ret = -EBUSY; + break; + } + continue; } + /* +* If we get a movable page, since we are going to be pinning +* these entries, try to move them out if possible. +*/ if (!is_pinnable_page(head)) { if (PageHuge(head)) { if (!isolate_huge_page(head, &movable_page_list)) @@ -1897,16 +1976,22 @@ static long check_and_migrate_movable_pages(unsig
[PATCH 1/3] migrate.c: Remove vma check in migrate_vma_setup()
migrate_vma_setup() checks that a valid vma is passed so that the page tables can be walked to find the pfns associated with a given address range. However in some cases the pfns are already known, such as when migrating device coherent pages during pin_user_pages() meaning a valid vma isn't required. Signed-off-by: Alistair Popple --- mm/migrate.c | 34 +- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index d3cc358..31ba8ca 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2581,24 +2581,24 @@ int migrate_vma_setup(struct migrate_vma *args) args->start &= PAGE_MASK; args->end &= PAGE_MASK; - if (!args->vma || is_vm_hugetlb_page(args->vma) || - (args->vma->vm_flags & VM_SPECIAL) || vma_is_dax(args->vma)) - return -EINVAL; - if (nr_pages <= 0) - return -EINVAL; - if (args->start < args->vma->vm_start || - args->start >= args->vma->vm_end) - return -EINVAL; - if (args->end <= args->vma->vm_start || args->end > args->vma->vm_end) - return -EINVAL; if (!args->src || !args->dst) return -EINVAL; - - memset(args->src, 0, sizeof(*args->src) * nr_pages); - args->cpages = 0; - args->npages = 0; - - migrate_vma_collect(args); + if (args->vma) { + if (is_vm_hugetlb_page(args->vma) || + (args->vma->vm_flags & VM_SPECIAL) || vma_is_dax(args->vma)) + return -EINVAL; + if (args->start < args->vma->vm_start || + args->start >= args->vma->vm_end) + return -EINVAL; + if (args->end <= args->vma->vm_start || args->end > args->vma->vm_end) + return -EINVAL; + + memset(args->src, 0, sizeof(*args->src) * nr_pages); + args->cpages = 0; + args->npages = 0; + + migrate_vma_collect(args); + } if (args->cpages) migrate_vma_unmap(args); @@ -2783,7 +2783,7 @@ void migrate_vma_pages(struct migrate_vma *migrate) continue; } - if (!page) { + if (!page && migrate->vma) { if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE)) continue; if (!notified) { -- git-series 0.9.1
[PATCH 0/3] Migrate device coherent pages on get_user_pages()
Device coherent pages represent memory on a coherently attached device such as a GPU which is usually under the control of a driver. These pages should not be pinned as the driver needs to be able to move pages as required. Currently this is enforced by failing any attempt to pin a device coherent page. A similar problem exists for ZONE_MOVABLE pages. In that case though the pages are migrated instead of causing failure. There is no reason the kernel can't migrate device coherent pages so this series implements migration for device coherent pages so the same strategy of migrate and pin can be used. This series depends on the series "Add MEMORY_DEVICE_COHERENT for coherent device memory mapping"[1] and should apply cleanly on top of that. [1] - https://lore.kernel.org/linux-mm/20220128200825.8623-1-alex.sie...@amd.com/ Alex Sierra (1): tools: add hmm gup test for long term pinned device pages Alistair Popple (2): migrate.c: Remove vma check in migrate_vma_setup() mm/gup.c: Migrate device coherent pages when pinning instead of failing mm/gup.c | 105 +++--- mm/migrate.c | 34 tools/testing/selftests/vm/Makefile| 2 +- tools/testing/selftests/vm/hmm-tests.c | 81 - 4 files changed, 194 insertions(+), 28 deletions(-) -- git-series 0.9.1
[PATCH] drm/i915/ttm: Return some errors instead of trying memcpy move
The i915_ttm_accel_move() function may return error codes that should be propagated further up the stack rather than consumed assuming that the accel move failed and could be replaced with a memcpy move. For -EINTR, -ERESTARTSYS and -EAGAIN, just propagate those codes, rather than retrying with a memcpy move. Fixes: 2b0a750caf33 ("drm/i915/ttm: Failsafe migration blits") Cc: Matthew Auld Signed-off-by: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c index 1de306c03aaf..1ebe6e4086a1 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c @@ -436,11 +436,17 @@ __i915_ttm_move(struct ttm_buffer_object *bo, if (!IS_ERR(fence)) goto out; - } else if (move_deps) { - int err = i915_deps_sync(move_deps, ctx); + } else { + int err = PTR_ERR(fence); + + if (err == -EINTR || err == -ERESTARTSYS || err == -EAGAIN) + return fence; - if (err) - return ERR_PTR(err); + if (move_deps) { + err = i915_deps_sync(move_deps, ctx); + if (err) + return ERR_PTR(err); + } } /* Error intercept failed or no accelerated migration to start with */ -- 2.34.1
[PATCH -next 2/2] video: fbdev: pxa3xx-gcu: Remove unnecessary print function dev_err()
The print function dev_err() is redundant because platform_get_irq() already prints an error. Eliminate the follow coccicheck warning: ./drivers/video/fbdev/pxa3xx-gcu.c:615:2-9: line 615 is redundant because platform_get_irq() already prints an error Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/video/fbdev/pxa3xx-gcu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c index 9239ecd34169..ea52c018bb51 100644 --- a/drivers/video/fbdev/pxa3xx-gcu.c +++ b/drivers/video/fbdev/pxa3xx-gcu.c @@ -612,7 +612,6 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev) /* request the IRQ */ irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(dev, "no IRQ defined: %d\n", irq); return irq; } -- 2.20.1.7.g153144c
[PATCH -next 1/2] video: fbdev: pxa168fb: Remove unnecessary print function dev_err()
The print function dev_err() is redundant because platform_get_irq() already prints an error. Eliminate the follow coccicheck warning: ./drivers/video/fbdev/pxa168fb.c:621:2-9: line 621 is redundant because platform_get_irq() already prints an error Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/video/fbdev/pxa168fb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/pxa168fb.c b/drivers/video/fbdev/pxa168fb.c index c25739f6934d..d533c0bf7031 100644 --- a/drivers/video/fbdev/pxa168fb.c +++ b/drivers/video/fbdev/pxa168fb.c @@ -618,7 +618,6 @@ static int pxa168fb_probe(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(&pdev->dev, "no IRQ defined\n"); return -ENOENT; } -- 2.20.1.7.g153144c
Re: [PATCH v2 3/5] ARM: dts: imx6qdl-vicut1: add CAN termination support
On Mon, Jan 31, 2022 at 04:39:04PM +0100, Sam Ravnborg wrote: > On Mon, Jan 31, 2022 at 11:28:39AM +0100, Oleksij Rempel wrote: > > The gpio1 0 pin is controlling CAN termination, not USB H1 VBUS. So, > > remove wrong regulator and assign this gpio to new DT CAN termnation > termination > > property. > > > > Signed-off-by: Oleksij Rempel > > I should be the last person to correct spelling mistakes, my commits > are always full of them - sigh. Thx! :) @Shawn, should I resend this patch set with fixed typo? Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 4/5] ARM: dts: imx6dl: plym2m, prtvt7, victgo: make use of new resistive-adc-touch driver
Hi Sam, On Mon, Jan 31, 2022 at 04:42:06PM +0100, Sam Ravnborg wrote: > On Mon, Jan 31, 2022 at 11:28:40AM +0100, Oleksij Rempel wrote: > > The tsc2046 is an ADC used as touchscreen controller. To share as mach > much > > code as possible, we should use it as actual ADC + virtual touchscreen > > controller. > > With this patch we make use of the new kernel IIO and HID infrastructure. > > > > Signed-off-by: Oleksij Rempel > > --- > > arch/arm/boot/dts/imx6dl-plym2m.dts | 59 + > > arch/arm/boot/dts/imx6dl-prtvt7.dts | 57 +--- > > arch/arm/boot/dts/imx6dl-victgo.dts | 59 ++--- > > 3 files changed, 132 insertions(+), 43 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx6dl-plym2m.dts > > b/arch/arm/boot/dts/imx6dl-plym2m.dts > > index 60fe5f14666e..73c7622bfe0f 100644 > > --- a/arch/arm/boot/dts/imx6dl-plym2m.dts > > +++ b/arch/arm/boot/dts/imx6dl-plym2m.dts > > @@ -101,6 +101,18 @@ reg_12v0: regulator-12v0 { > > regulator-min-microvolt = <1200>; > > regulator-max-microvolt = <1200>; > > }; > > + > > + touchscreen { > > + compatible = "resistive-adc-touch"; > > + io-channels = <&adc_ts 1>, <&adc_ts 3>, <&adc_ts 4>, > > + <&adc_ts 5>; > > + io-channel-names = "y", "z1", "z2", "x"; > > + touchscreen-min-pressure = <64687>; > > + touchscreen-inverted-x; > > + touchscreen-inverted-y; > > + touchscreen-x-plate-ohms = <300>; > > + touchscreen-y-plate-ohms = <800>; > > + }; > > }; > > > > &can1 { > > @@ -129,26 +141,41 @@ &ecspi2 { > > pinctrl-0 = <&pinctrl_ecspi2>; > > status = "okay"; > > > > - touchscreen@0 { > > - compatible = "ti,tsc2046"; > > + adc_ts: adc@0 { > > + compatible = "ti,tsc2046e-adc"; > > reg = <0>; > > pinctrl-0 = <&pinctrl_tsc2046>; > > pinctrl-names ="default"; > > - spi-max-frequency = <10>; > > - interrupts-extended = <&gpio3 20 IRQ_TYPE_EDGE_FALLING>; > > - pendown-gpio = <&gpio3 20 GPIO_ACTIVE_LOW>; > > + spi-max-frequency = <100>; > > + interrupts-extended = <&gpio3 20 IRQ_TYPE_LEVEL_LOW>; > > + #io-channel-cells = <1>; > > I quickly skimmed the patch - we seem to loose the pendown-gpio in most > of the patches - I do not see it replaced. pendown-gpio is not used by the new driver. It is replace by the IIO trigger. Please see this comment: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/iio/adc/ti-tsc2046.c?h=testing#n521 Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
RE: [PATCH 1/3] drm: add writeback pointers to drm_connector
Hey, > I think there are more places affected with this change. I can get below > compilation issues while trying to compile my branch: > > drivers/gpu/drm/vc4/vc4_txp.c: In function ‘encoder_to_vc4_txp’: > ./include/linux/build_bug.h:78:41: error: static assertion failed: > "pointer type mismatch in container_of()" > #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) > ^ > ./include/linux/build_bug.h:77:34: note: in expansion of macro > ‘__static_assert’ > #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, > #expr) >^ > ./include/linux/container_of.h:19:2: note: in expansion of macro > ‘static_assert’ >static_assert(__same_type(*(ptr), ((type *)0)->member) || \ >^ > drivers/gpu/drm/vc4/vc4_txp.c:162:9: note: in expansion of macro > ‘container_of’ >return container_of(encoder, struct vc4_txp, connector.encoder); > ^ > drivers/gpu/drm/vc4/vc4_txp.c: In function ‘connector_to_vc4_txp’: > ./include/linux/build_bug.h:78:41: error: static assertion failed: > "pointer type mismatch in container_of()" > #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) > ^ > ./include/linux/build_bug.h:77:34: note: in expansion of macro > ‘__static_assert’ > #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, > #expr) >^ > ./include/linux/container_of.h:19:2: note: in expansion of macro > ‘static_assert’ >static_assert(__same_type(*(ptr), ((type *)0)->member) || \ >^ > drivers/gpu/drm/vc4/vc4_txp.c:167:9: note: in expansion of macro > ‘container_of’ >return container_of(conn, struct vc4_txp, connector.base); > ^ > drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_bind’: > drivers/gpu/drm/vc4/vc4_txp.c:495:27: error: passing argument 1 of > ‘drm_connector_helper_add’ from incompatible pointer type [- > Werror=incompatible-pointer-types] >drm_connector_helper_add(&txp->connector.base, > ^ > In file included from ./include/drm/drm_atomic_helper.h:32:0, > from drivers/gpu/drm/vc4/vc4_txp.c:17: > ./include/drm/drm_modeset_helper_vtables.h:1153:20: note: expected > ‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’ > static inline void drm_connector_helper_add(struct drm_connector > *connector, > ^ > drivers/gpu/drm/vc4/vc4_txp.c:509:10: error: assignment from incompatible > pointer type [-Werror=incompatible-pointer-types] >encoder = &txp->connector.encoder; >^ > drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_unbind’: > drivers/gpu/drm/vc4/vc4_txp.c:532:28: error: passing argument 1 of > ‘vc4_txp_connector_destroy’ from incompatible pointer type [- > Werror=incompatible-pointer-types] >vc4_txp_connector_destroy(&txp->connector.base); > ^ > drivers/gpu/drm/vc4/vc4_txp.c:333:13: note: expected ‘struct > drm_connector *’ but argument is of type ‘struct drm_connector **’ > static void vc4_txp_connector_destroy(struct drm_connector *connector) > I will have a look at this and try to resolve it > > Looks like vc4 doesnt have its own encoder so we might have to move it to > have its own? Right seems like we will have to add a drm_connector and drm_encoder in the below structure > > struct vc4_txp { > struct vc4_crtc base; > > struct platform_device *pdev; > > struct drm_writeback_connector connector; > > void __iomem *regs; > struct debugfs_regset32 regset; > }; > > static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder > *encoder) > { > return container_of(encoder, struct vc4_txp, connector.encoder); } > > static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector > *conn) > { > return container_of(conn, struct vc4_txp, connector.base); } > Changes will be required here in both connector_of functions to point to The new drm_connector and drm_encoder we add in vc4_txp struct. > > One more thing, it looks like to me, we need to do one more thing. > > Like intel does and what MSM will do, the encoder pointer of the wb > connector has to point to the encoder struct . > > >> wb_conn = &intel_connector->wb_conn; > >> wb_conn->base = &intel_connector->base; > >> wb_conn->encoder = &intel_wd->base.base; > Yes this will need to be done Thanks Abhinav for pointing Ill implement this and send it in the next version of patches Regards, Suraj Kandpal
Re: (subset) [PATCH 0/4] Enable display backlight on Fairphone 4
On Wed, 29 Dec 2021 18:03:54 +0100, Luca Weiss wrote: > Add and enable PM6150L wled which is used for controlling the display > backlight on Fairphone 4. > > This series depends on the recent wled series by Marijn Suijten, > currently applied in the for-backlight-next branch of > kernel/git/lee/backlight.git (or linux-next). > > [...] Applied, thanks! [3/4] arm64: dts: qcom: pm6150l: Add wled node commit: fe508ced49dd51a700c0f9ec7826d523cfe621b2 [4/4] arm64: dts: qcom: sm7225-fairphone-fp4: Configure WLED commit: 7a52967d9050f3e430373bc51c56865b49a38573 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 6/6] arm64: dts: qcom: sdm845: add device tree for SHIFT6mq
On Sun, 23 Jan 2022 17:38:15 +, Caleb Connolly wrote: > From: Alexander Martinz > > Add initial support for the SHIFT SHIFT6mq (axolotl) based on > the sdm845-mtp DT. > > Currently supported features: > * Buttons (power, volume) > * Bluetooth, DSPs and modem > * Display and GPU > * Touch > * UART > * USB peripheral mode > * WLAN > > [...] Applied, thanks! [6/6] arm64: dts: qcom: sdm845: add device tree for SHIFT6mq commit: 45882459159deb792718786514bc677c8a6b1f53 Best regards, -- Bjorn Andersson
Re: [Freedreno] [PATCH v2 4/7] drm/msm/dpu: allow just single IRQ callback
On 19/01/2022 03:10, abhin...@codeaurora.org wrote: On 2021-08-17 20:30, abhin...@codeaurora.org wrote: On 2021-06-17 15:20, Dmitry Baryshkov wrote: DPU interrupts code allows multiple callbacks per interrut. In reality /interrupt none of the interrupts is shared between blocks (and will probably never be). Drop support for registering multiple callbacks per interrupt to simplify interrupt handling code. Signed-off-by: Dmitry Baryshkov I need to check on why we had this design originally and we still do. the idx with which we are registering today can generate only one hw interrupt. But i am not sure if something for planned for future use. Will update in a day or two. meanwhile some comments and questions below. I did check internally on the original design of this and yes the plan was for other sub-modules to register for callbacks and that callback goes into the callback list. For example, lets say for some reason some other modules like the DSI want to register for the VSYNC interrupt, it can register for a callback and that will get added to the callback list. But, this never got used that way and even now there is only one callback per interrupt. We dont have a concrete use-case where we will need this in the future but like many other things, we cannot tell for certain. Since there is no concrete use-case where we might need this, if you would like to go ahead with this, please do. Once you address the other comments on this, I can ack this. Thanks for the info. I'll respin this on top of current msm-next + https://patchwork.freedesktop.org/patch/464350/ --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 18 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 144 +++--- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 12 -- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- 9 files changed, 86 insertions(+), 134 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h index 90ae6c9ccc95..44ab97fb2964 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h @@ -46,10 +46,8 @@ u32 dpu_core_irq_read( * interrupt * @dpu_kms: DPU handle * @irq_idx: irq index - * @irq_cb: IRQ callback structure, containing callback function - * and argument. Passing NULL for irq_cb will unregister - * the callback for the given irq_idx - * This must exist until un-registration. + * @irq_cb: IRQ callback funcion. + * @irq_arg: IRQ callback argument. * @return: 0 for success registering callback, otherwise failure * * This function supports registration of multiple callbacks for each interrupt. @@ -57,17 +55,16 @@ u32 dpu_core_irq_read( int dpu_core_irq_register_callback( struct dpu_kms *dpu_kms, int irq_idx, - struct dpu_irq_callback *irq_cb); + void (*irq_cb)(void *arg, int irq_idx), + void *irq_arg); /** * dpu_core_irq_unregister_callback - For unregistering callback function on IRQ * interrupt * @dpu_kms: DPU handle * @irq_idx: irq index - * @irq_cb: IRQ callback structure, containing callback function - * and argument. Passing NULL for irq_cb will unregister - * the callback for the given irq_idx - * This must match with registration. + * @irq_cb: IRQ callback funcion. /function this typo is there in multiple places + * @irq_arg: IRQ callback argument. * @return: 0 for success registering callback, otherwise failure * * This function supports registration of multiple callbacks for each interrupt. @@ -75,7 +72,8 @@ int dpu_core_irq_register_callback( int dpu_core_irq_unregister_callback( struct dpu_kms *dpu_kms, int irq_idx, - struct dpu_irq_callback *irq_cb); + void (*irq_cb)(void *arg, int irq_idx), + void *irq_arg); /** * dpu_debugfs_core_irq_init - register core irq debugfs diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1c04b7cce43e..d3557b0f4db9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -310,7 +310,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc, phys_enc->hw_pp->idx - PINGPONG_0, atomic_read(wait_info->atomic_cnt)); local_irq_save(flags); - irq->cb.func(phys_enc, irq->irq_idx); + irq->func(phys_enc, irq->irq_idx);
Re: [PATCH] drm/msm/dp: add wide bus support
On Tue, 1 Feb 2022 at 06:16, Dmitry Baryshkov wrote: > > On Tue, 1 Feb 2022 at 01:24, Kuogee Hsieh wrote: > > > > > > On 1/28/2022 8:59 PM, Dmitry Baryshkov wrote: > > > Hi, > > > > > > Thank you for your patch. > > > > > > On Fri, 28 Jan 2022 at 20:29, Kuogee Hsieh > > > wrote: > > >> Normally, mdp will push one pixel of data per pixel clock to > > >> interface to display. Wide bus feature will increase bus > > >> width from 32 bits to 64 bits so that it can push two > > >> pixel of data per pixel clock to interface to display. > > >> This feature is pre requirement to support 4k resolution. > > >> > > >> Signed-off-by: Kuogee Hsieh > > >> --- > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 16 ++- > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 + > > >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++ > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 108 > > >> +++-- > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 5 + > > >> drivers/gpu/drm/msm/dp/dp_catalog.c| 11 ++- > > >> drivers/gpu/drm/msm/dp/dp_catalog.h| 1 + > > >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +- > > >> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + > > >> drivers/gpu/drm/msm/dp/dp_display.c| 17 > > >> drivers/gpu/drm/msm/dp/dp_display.h| 3 + > > >> drivers/gpu/drm/msm/dp/dp_parser.c | 26 + > > >> drivers/gpu/drm/msm/dp/dp_parser.h | 2 + > > >> drivers/gpu/drm/msm/msm_drv.h | 9 ++ > > > Can we get this split into logical chunks please? > > > > > >> 14 files changed, 190 insertions(+), 34 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > >> index 1e648db..e2fb5bc 100644 > > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > >> @@ -199,6 +199,8 @@ struct dpu_encoder_virt { > > >> > > >> struct msm_display_info disp_info; > > >> > > >> + struct msm_op_info op_info; > > >> + > > >> bool idle_pc_supported; > > >> struct mutex rc_lock; > > >> enum dpu_enc_rc_states rc_state; > > >> @@ -217,6 +219,13 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { > > >> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 > > >> }; > > >> > > >> +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc) > > >> +{ > > >> + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > > >> + > > >> + return dpu_enc->op_info.wide_bus_en; > > >> +} > > >> + > > >> static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, > > >> unsigned bpc) > > >> { > > >> struct dpu_hw_dither_cfg dither_cfg = { 0 }; > > >> @@ -2112,6 +2121,7 @@ int dpu_encoder_setup(struct drm_device *dev, > > >> struct drm_encoder *enc, > > >> struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > > >> struct drm_encoder *drm_enc = NULL; > > >> struct dpu_encoder_virt *dpu_enc = NULL; > > >> + struct msm_op_info *op_info; > > >> int ret = 0; > > >> > > >> dpu_enc = to_dpu_encoder_virt(enc); > > >> @@ -2128,8 +2138,12 @@ int dpu_encoder_setup(struct drm_device *dev, > > >> struct drm_encoder *enc, > > >> timer_setup(&dpu_enc->vsync_event_timer, > > >> dpu_encoder_vsync_event_handler, > > >> 0); > > >> - else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) > > >> + else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) { > > >> dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]]; > > >> + op_info = &priv->op_info[disp_info->h_tile_instance[0]]; > > > op_info should be defined per INTF rather than per h_tile. This way > > > you won't have to check for intf_type here. > > > > The more i am thinking, the more i think op_info should be along with > > controller. > > > > widebus belongs to mdp fetching process and intf only care of raw bit > > stream. it has nothing to do with intf. > > Technically yes. However having it per INTF (or per encoder) might > simplify things (as you won't have to check for the encoder type in > the code afterwards). > I'd say, you can define it inside msm_dp somewhere and then query it > from dpu_encoder_setup() using the new function. Afterwards you don't > have to worry about the INTF type at all. You have the correct value > for DP and for the DSI it's false. After even more thought, a slightly different suggestion: add wide_bus_enabled to struct msm_display_info and set it in the _dpu_kms_initialize_displayport(). > > > > > The other problem is we can not access inside of prov->dp here. > > You can always add msm_dp_has_widebus_enabled() function. > > > > >
Re: [PATCH] drm/msm/dp: add wide bus support
On Fri 28 Jan 09:29 PST 2022, Kuogee Hsieh wrote: > Normally, mdp will push one pixel of data per pixel clock to > interface to display. Wide bus feature will increase bus > width from 32 bits to 64 bits so that it can push two > pixel of data per pixel clock to interface to display. > This feature is pre requirement to support 4k resolution. > Can you please elaborate further how this is a requirement for 4k resolution, given that I write you this question on a 4k monitor. Regards, Bjorn
Re: [PATCH] drm/msm/dp: add wide bus support
On Tue, 1 Feb 2022 at 01:24, Kuogee Hsieh wrote: > > > On 1/28/2022 8:59 PM, Dmitry Baryshkov wrote: > > Hi, > > > > Thank you for your patch. > > > > On Fri, 28 Jan 2022 at 20:29, Kuogee Hsieh wrote: > >> Normally, mdp will push one pixel of data per pixel clock to > >> interface to display. Wide bus feature will increase bus > >> width from 32 bits to 64 bits so that it can push two > >> pixel of data per pixel clock to interface to display. > >> This feature is pre requirement to support 4k resolution. > >> > >> Signed-off-by: Kuogee Hsieh > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 16 ++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 + > >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 108 > >> +++-- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 5 + > >> drivers/gpu/drm/msm/dp/dp_catalog.c| 11 ++- > >> drivers/gpu/drm/msm/dp/dp_catalog.h| 1 + > >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +- > >> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + > >> drivers/gpu/drm/msm/dp/dp_display.c| 17 > >> drivers/gpu/drm/msm/dp/dp_display.h| 3 + > >> drivers/gpu/drm/msm/dp/dp_parser.c | 26 + > >> drivers/gpu/drm/msm/dp/dp_parser.h | 2 + > >> drivers/gpu/drm/msm/msm_drv.h | 9 ++ > > Can we get this split into logical chunks please? > > > >> 14 files changed, 190 insertions(+), 34 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> index 1e648db..e2fb5bc 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> @@ -199,6 +199,8 @@ struct dpu_encoder_virt { > >> > >> struct msm_display_info disp_info; > >> > >> + struct msm_op_info op_info; > >> + > >> bool idle_pc_supported; > >> struct mutex rc_lock; > >> enum dpu_enc_rc_states rc_state; > >> @@ -217,6 +219,13 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { > >> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 > >> }; > >> > >> +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc) > >> +{ > >> + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); > >> + > >> + return dpu_enc->op_info.wide_bus_en; > >> +} > >> + > >> static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, > >> unsigned bpc) > >> { > >> struct dpu_hw_dither_cfg dither_cfg = { 0 }; > >> @@ -2112,6 +2121,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct > >> drm_encoder *enc, > >> struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > >> struct drm_encoder *drm_enc = NULL; > >> struct dpu_encoder_virt *dpu_enc = NULL; > >> + struct msm_op_info *op_info; > >> int ret = 0; > >> > >> dpu_enc = to_dpu_encoder_virt(enc); > >> @@ -2128,8 +2138,12 @@ int dpu_encoder_setup(struct drm_device *dev, > >> struct drm_encoder *enc, > >> timer_setup(&dpu_enc->vsync_event_timer, > >> dpu_encoder_vsync_event_handler, > >> 0); > >> - else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) > >> + else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) { > >> dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]]; > >> + op_info = &priv->op_info[disp_info->h_tile_instance[0]]; > > op_info should be defined per INTF rather than per h_tile. This way > > you won't have to check for intf_type here. > > The more i am thinking, the more i think op_info should be along with > controller. > > widebus belongs to mdp fetching process and intf only care of raw bit > stream. it has nothing to do with intf. Technically yes. However having it per INTF (or per encoder) might simplify things (as you won't have to check for the encoder type in the code afterwards). I'd say, you can define it inside msm_dp somewhere and then query it from dpu_encoder_setup() using the new function. Afterwards you don't have to worry about the INTF type at all. You have the correct value for DP and for the DSI it's false. > > The other problem is we can not access inside of prov->dp here. You can always add msm_dp_has_widebus_enabled() function. > > > > > > >> + dpu_enc->op_info = *op_info; > > So... we set this data in msm_drm_private only to copy it to > > dpu_encoder_virt? Please don't do this. > > Allow one to query the data from the DP rather than blindly copying it > > over and over again. > > Is this only copy one time? maybe change to pointer work better? This is the only time we copy op_info, this is true. The wide_bus_en is copied 5 times, if I'm not
[PATCH v2] fbdev: fbmem: Fix the implicit type casting
In function do_fb_ioctl(), the "arg" is the type of unsigned long, and in "case FBIOBLANK:" this argument is casted into an int before passig to fb_blank(). In fb_blank(), the comparision if (blank > FB_BLANK_POWERDOWN) would be bypass if the original "arg" is a large number, which is possible because it comes from the user input. Fix this by adding the check before the function call. Signed-off-by: Yizhuo Zhai --- drivers/video/fbdev/core/fbmem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..f08326efff54 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1162,6 +1162,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOBLANK: console_lock(); lock_fb_info(info); + if (blank > FB_BLANK_POWERDOWN) + blank = FB_BLANK_POWERDOWN; ret = fb_blank(info, arg); /* might again call into fb_blank */ fbcon_fb_blanked(info, arg); -- 2.25.1
Re: [PATCH v2] drm/msm/dp: add connector type to enhance debug messages
On 27/01/2022 02:46, Kuogee Hsieh wrote: DP driver is a generic driver which supports both eDP and DP. For debugging purpose it is required to have capabilities to differentiate message are generated from eDP or DP. This patch do: 1) add connector type into debug messages within dp_display.c 2) revise debug messages related to DP phy within dp_ctrl.c 3) replace DRM_DEBUG_DP marco with drm_dbg_dp You list three items here. This patch should be split into respective three parts. Changes in V2: -- replace DRM_DEBUG_DP marco with drm_dbg_dp I suppose that Stephen's idea was to pass proper drm_device to this function rather than always passing NULL. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_audio.c | 49 +-- drivers/gpu/drm/msm/dp/dp_catalog.c | 34 ++- drivers/gpu/drm/msm/dp/dp_ctrl.c| 116 +++- drivers/gpu/drm/msm/dp/dp_display.c | 103 ++-- drivers/gpu/drm/msm/dp/dp_drm.c | 4 +- drivers/gpu/drm/msm/dp/dp_link.c| 99 +- drivers/gpu/drm/msm/dp/dp_panel.c | 43 +++-- drivers/gpu/drm/msm/dp/dp_parser.c | 2 +- drivers/gpu/drm/msm/dp/dp_power.c | 20 --- 9 files changed, 283 insertions(+), 187 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c b/drivers/gpu/drm/msm/dp/dp_audio.c index d7e4a39..4fbbe0a 100644 --- a/drivers/gpu/drm/msm/dp/dp_audio.c +++ b/drivers/gpu/drm/msm/dp/dp_audio.c @@ -136,7 +136,8 @@ static void dp_audio_stream_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_1_BIT) | (parity_byte << PARITY_BYTE_1_BIT)); - DRM_DEBUG_DP("Header Byte 1: value = 0x%x, parity_byte = 0x%x\n", + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 1: value = 0x%x, parity_byte = 0x%x\n", value, parity_byte); dp_audio_set_header(catalog, value, DP_AUDIO_SDP_STREAM, DP_AUDIO_SDP_HEADER_1); @@ -148,7 +149,8 @@ static void dp_audio_stream_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_2_BIT) | (parity_byte << PARITY_BYTE_2_BIT)); - DRM_DEBUG_DP("Header Byte 2: value = 0x%x, parity_byte = 0x%x\n", + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 2: value = 0x%x, parity_byte = 0x%x\n", value, parity_byte); dp_audio_set_header(catalog, value, @@ -162,7 +164,8 @@ static void dp_audio_stream_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_3_BIT) | (parity_byte << PARITY_BYTE_3_BIT)); - DRM_DEBUG_DP("Header Byte 3: value = 0x%x, parity_byte = 0x%x\n", + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 3: value = 0x%x, parity_byte = 0x%x\n", value, parity_byte); dp_audio_set_header(catalog, value, @@ -183,8 +186,9 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_1_BIT) | (parity_byte << PARITY_BYTE_1_BIT)); - DRM_DEBUG_DP("Header Byte 1: value = 0x%x, parity_byte = 0x%x\n", - value, parity_byte); + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 1: value = 0x%x, parity_byte = 0x%x\n", + value, parity_byte); dp_audio_set_header(catalog, value, DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_1); @@ -196,7 +200,8 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_2_BIT) | (parity_byte << PARITY_BYTE_2_BIT)); - DRM_DEBUG_DP("Header Byte 2: value = 0x%x, parity_byte = 0x%x\n", + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 2: value = 0x%x, parity_byte = 0x%x\n", value, parity_byte); dp_audio_set_header(catalog, value, DP_AUDIO_SDP_TIMESTAMP, DP_AUDIO_SDP_HEADER_2); @@ -209,7 +214,8 @@ static void dp_audio_timestamp_sdp(struct dp_audio_private *audio) parity_byte = dp_audio_calculate_parity(new_value); value |= ((new_value << HEADER_BYTE_3_BIT) | (parity_byte << PARITY_BYTE_3_BIT)); - DRM_DEBUG_DP("Header Byte 3: value = 0x%x, parity_byte = 0x%x\n", + drm_dbg_dp((struct drm_device *)NULL, + "Header Byte 3: value = 0x%x, parity_byte = 0x%x\n", value, parity_byte); dp_audio_set_header(catalog, val
Re: [PATCH 1/3] drm: add writeback pointers to drm_connector
Hi Suraj I think there are more places affected with this change. I can get below compilation issues while trying to compile my branch: drivers/gpu/drm/vc4/vc4_txp.c: In function ‘encoder_to_vc4_txp’: ./include/linux/build_bug.h:78:41: error: static assertion failed: "pointer type mismatch in container_of()" #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) ^ ./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’ #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) ^ ./include/linux/container_of.h:19:2: note: in expansion of macro ‘static_assert’ static_assert(__same_type(*(ptr), ((type *)0)->member) || \ ^ drivers/gpu/drm/vc4/vc4_txp.c:162:9: note: in expansion of macro ‘container_of’ return container_of(encoder, struct vc4_txp, connector.encoder); ^ drivers/gpu/drm/vc4/vc4_txp.c: In function ‘connector_to_vc4_txp’: ./include/linux/build_bug.h:78:41: error: static assertion failed: "pointer type mismatch in container_of()" #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) ^ ./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’ #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) ^ ./include/linux/container_of.h:19:2: note: in expansion of macro ‘static_assert’ static_assert(__same_type(*(ptr), ((type *)0)->member) || \ ^ drivers/gpu/drm/vc4/vc4_txp.c:167:9: note: in expansion of macro ‘container_of’ return container_of(conn, struct vc4_txp, connector.base); ^ drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_bind’: drivers/gpu/drm/vc4/vc4_txp.c:495:27: error: passing argument 1 of ‘drm_connector_helper_add’ from incompatible pointer type [-Werror=incompatible-pointer-types] drm_connector_helper_add(&txp->connector.base, ^ In file included from ./include/drm/drm_atomic_helper.h:32:0, from drivers/gpu/drm/vc4/vc4_txp.c:17: ./include/drm/drm_modeset_helper_vtables.h:1153:20: note: expected ‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’ static inline void drm_connector_helper_add(struct drm_connector *connector, ^ drivers/gpu/drm/vc4/vc4_txp.c:509:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types] encoder = &txp->connector.encoder; ^ drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_unbind’: drivers/gpu/drm/vc4/vc4_txp.c:532:28: error: passing argument 1 of ‘vc4_txp_connector_destroy’ from incompatible pointer type [-Werror=incompatible-pointer-types] vc4_txp_connector_destroy(&txp->connector.base); ^ drivers/gpu/drm/vc4/vc4_txp.c:333:13: note: expected ‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’ static void vc4_txp_connector_destroy(struct drm_connector *connector) Looks like vc4 doesnt have its own encoder so we might have to move it to have its own? struct vc4_txp { struct vc4_crtc base; struct platform_device *pdev; struct drm_writeback_connector connector; void __iomem *regs; struct debugfs_regset32 regset; }; static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder) { return container_of(encoder, struct vc4_txp, connector.encoder); } static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn) { return container_of(conn, struct vc4_txp, connector.base); } One more thing, it looks like to me, we need to do one more thing. Like intel does and what MSM will do, the encoder pointer of the wb connector has to point to the encoder struct . >> wb_conn = &intel_connector->wb_conn; >> wb_conn->base = &intel_connector->base; >> wb_conn->encoder = &intel_wd->base.base; Thanks Abhinav On 1/27/2022 10:17 AM, Abhinav Kumar wrote: Hi Suraj Thanks for the response. I was not too worried about the intel driver as I am sure you must have validated this change with that :) My question was more for the other vendor writeback drivers. Thanks for looking into the others and providing the snippets. After looking at those, yes I also think it should work. So, if others do not have any concern with this change, Reviewed-by: Abhinav Kumar On 1/27/2022 1:33 AM, Kandpal, Suraj wrote: + laurent on this Hi Suraj Jani pointed me to this thread as i had posted something similar here : https://patchwork.freedesktop.org/patch/470296/ but since this thread was posted earlier, we can discuss further here. Overall, its similar to what I had posted in the RFC and your commit text also covers my concerns too. One question I have about your change is since you have changed wb_connector::encoder to be a pointer, i saw the other changes in the series but they do not allocate an encoder
Re: [PATCH 00/14] Rename dma-buf-map
On Fri, Jan 28, 2022 at 10:48:42AM +0100, Christian König wrote: Am 28.01.22 um 10:40 schrieb Lucas De Marchi: On Fri, Jan 28, 2022 at 10:22:00AM +0100, Christian König wrote: Am 28.01.22 um 10:12 schrieb Lucas De Marchi: On Fri, Jan 28, 2022 at 09:41:14AM +0100, Christian König wrote: Rule #1 is to never ever break the build. Because of this all those patches needs to be squashed into a single one as far as I can see. what config are you building on? Well I'm not building at all, I'm just looking at the patches as an engineer with 25 years of experience with Linux patches. Just take a look at patch number 2: -static int fastrpc_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map) +static int fastrpc_vmap(struct dma_buf *dmabuf, struct iosys_map *map) You are changing the functions signature without changing any of the callers. At bare minimum that causes a warning and on runtime this only works by coincident now because the structure pointers just happen to have the same layout. This is not something we usually do. you missed the magic/hack on patch 1: 1) dma-buf-map.h includes iosys-map.h _at the end_ 2) iosys-map.h includes dma-buf-map.h at the beginning and initially does a "define iosys_map dma_buf_map". So, it doesn't work by coincidence, It's because it was done to allow converting it piecemeal. Oh, my. Please never do stuff like that again. It's not uncommon approach to be required by other subsystems. Even drm-intel already used similar approach for macro conversions crossing drm-intel-next and drm-intel-gt-next branches recently. As I said, I don't mind one way or the other. Before I go and respin this into a single mega patch, I'd like to gather some feedback on the following topics: 1) Daniel Vetter and Thomas Zimmermann seemed to be ok with staying with the current name, dma_buf_map, while you prefer it renamed. Or at least not make the rename a pre-requisite for the API additions in https://lore.kernel.org/all/20220126203702.1784589-1-lucas.demar...@intel.com/ One thing I like about the rename is that it makes clear the separation between this small shim and dma-buf. There are also some APIs that are really dma-buf API (e.g. dma_buf_map_attachment()), but if you don't look carefully you may think it's from dma_buf_map. 2) If renaming, would it still keep the same entry in MAINTAINERS? Thomas suggested drivers core, but this all seem to be used mainly on drm/, with just one exception. 3) If renaming, do we have another preferred name? thanks Lucas De Marchi But as I said, I don't really have a preference. When crossing subsystems one thing that is hard is that different people have different preferences on these things. At least squashing now is much easier than if I had to split it Try to imagine how much complain I received on going the other way in 25985edcedea6396277003854657b5f3cb31a628 with 2463 files changed, 4252 insertions(+), 4252 deletions(-) Well exactly that is perfectly fine. What you do here is applying your personal hack which is absolutely not welcomed. Regards, Christian. :) Lucas De Marchi Regards, Christian. I built this series, full config with CONFIG_COMPILE_TEST and doing: git rebase -i -x "make -j$(nproc)" I split these patches in a way that wouldn't break the build on purpose. There were a couple that I couldn't build without cross compiling: tegra and rockchip. The others were ok. I'm not really against squashing everything in one to merge, though. It will be hard on the conflicts later, but should get the job done much quicker. Lucas De Marchi Regards, Christian. Am 28.01.22 um 09:36 schrieb Lucas De Marchi: Motivation for this started in https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220126203702.1784589-1-lucas.demarchi%40intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C635084a520994d35a16e08d9e2423319%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637789596221829397%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ruHpD3DbyyqQuZIFEQU%2B2RH31OwsdFnn1v7N4z75U0Y%3D&reserved=0 when trying to extend the dma-buf-map API to cover new use cases: help a single driver with allocations and sharing code paths for IO and system memory. I'm leaving the API additions aside and first renaming the interface as requested. There are already some users in tree outside the context of dma-buf importer/exporter. So before extending the API, let's dissociate it from dma-buf. The iosys-map.h is introduced in the first patch in a way that allows the conversion of each driver to happen separately. After all the conversions are done we can remove the old one, which is the last patch. Another possible way is to squash everything and merge together, but I believe this would make much harder for review. The conversion was done with the following semantic patch: @r1@ @@ - struct dma_
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hello Sam, Thanks for your suggestions. On 1/31/22 22:30, Sam Ravnborg wrote: > Hi Javier, > > On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: >> Add a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED >> controllers that can be programmed via an I2C interface. This is a port >> of the ssd1307fb driver that already supports these devices. >> >> A Device Tree binding is not added because the DRM driver is compatible >> with the existing binding for the ssd1307fb driver. > > The driver uses the pwms property for the backlight. > It would have been much better to bite the bullet and require the > backlight to be specified using a backlight node in the DT. > This is the standard way to do it and then the driver could use the > existing backlight driver rather than embedding a backlight driver here. > I did consider that. Because that would allow me to use a struct drm_panel and as you said make the core to manage the backlight. But then decied to just keep the backward compatibility with the existing binding to make it easier for users to migrate to the DRM driver. I wonder if we could make the driver to support both ? That is, to query if there's a backlight with drm_panel_of_backlight() and if not found then registering it's own backlight like the driver is currently doing. > It would cost some backward compatibility - but looking forward this is > the right thing to do. > I don't have a strong opinion. As mentioned I thought that was more worthy to keep the backward compat over doing it correctly. After all, this driver is for a very niche hardware (small OLED monochromatic panels), so I wasn't that concerned about supporting the proper DT conventions. I would also like to know Andy's opinion. Since he mentioned that embedded x86/ACPI hardware is using the PRP0001 _DSD extension to match against the OF driver. I wonder how feasible is for those platforms to change the DSDT. Another option is to have different compatible strings for this ? I wanted to drop the "fb" suffix and just have OF entries with compatible "ssd1307" instead of "ssd1307fb", but again left the latter for backward compat. But maybe we can have the "fb" ones creating the backlight and the new ones expecting it to have a backlight DT node. >> >> Signed-off-by: Javier Martinez Canillas > > Some random comments in the following. > > Sam [snip] >> +config TINYDRM_SSD1307 >> +tristate "DRM support for Solomon SSD1307 OLED displays" > Use SSD130X here - so SSD1306 users can find it. > Agreed. That's what I had before (and also in the driver, MAINTAINERS entry, etc) but then changed to SSD1307 everywhere. Unsure what could be more clear to users grepping to figure out if there's a DRM driver for their device. I'll follow your suggestion and change in the Kconfig description for v2. [snip] >> + >> +static int ssd1307_fb_blit_rect(struct drm_framebuffer *fb, const struct >> dma_buf_map *map, >> +struct drm_rect *rect) >> +{ >> +struct ssd1307_device *ssd1307 = drm_to_ssd1307(fb->dev); >> +void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ > Hmm, can we fix this TODO now? > I did try to fix it while writing the driver but couldn't figure out what was the proper abstraction to use. Looked at other drivers but *many* do the same and have the exact same comment :) $ git grep "TODO: Use mapping abstraction properly" -- drivers/gpu/ | wc -l 15 I'll be happy to fix it if someone could elaborate what this comment expects. [snip] >> + >> +if (!drm_dev_enter(drm, &idx)) >> +return; >> + >> +backlight_enable(ssd1307->bl_dev); > This seems backward - I would have assumed backlight was enabled > after the display was enabled - so you potentially hide some flickering. > This is the order done in drm_panel. > I see. Sure, I'll change this in v2 as well. [snip] >> + >> +backlight_disable(ssd1307->bl_dev); > And here backlight could be disabled before disabling the display. > Ok. [snip] >> + >> +/* Turn on the display */ >> +ret = ssd1307_write_cmd(ssd1307->client, SSD1307_DISPLAY_ON); >> +if (ret < 0) >> +return ret; > I would assume the display is turned on later - and should be left off > here. The pipe function will do this when needed. > Indeed. I based this on the the ssd1307fb probe logi but forgot to remove this that's not needed since will be managed by display pipe {en,dis}able. >> + >> +return 0; >> +} >> + >> +static int ssd1307_update_bl(struct backlight_device *bdev) >> +{ >> +struct ssd1307_device *ssd1307 = bl_get_data(bdev); >> +int brightness = bdev->props.brightness; > Use backlight_get_brightness(bdev) here. > Ok. >> +int ret; >> + >> +ssd1307->contrast = brightness; >> + >> +ret = ssd1307_write_cmd(ssd1307->client, SSD1307_CONTRAST); >> +if (ret < 0) >> +return ret; >> + >> +ret = ssd1307_write_cmd(ssd1307-
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On 1/31/22 21:56, Sam Ravnborg wrote: > Hi Javier, > On Mon, Jan 31, 2022 at 09:12:20PM +0100, Javier Martinez Canillas wrote: >> This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, >> SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. >> >> Using the DRM fb emulation, all the tests from Geert Uytterhoeven's fbtest >> (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes: > > Impressed how fast you did this! > Saw the picture you posted a link to on irc - nice. > Thanks :) What's impressive is how many helper functions the DRM core has, so typing a new DRM driver is something that could be achieved in a few hours. Which was one of my goals with this experiment, to understand how much effort would be for a developer with no prior experience with DRM to port a fbdev driver. >> Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x >> (which would be more accurate) to avoid confusion for users who want to >> migrate from the existing ssd1307fb fbdev driver. > Looking forward the name ssd130x would make more sense. There is only so > many existing users and a potential of much more new users. > So in my color of the world the naming that benefits the most users > wins. > Agreed. That's also what Andy suggested and makes a lot of sense to me. > Sam > Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v5 09/10] tools: update hmm-test to support device coherent type
Oh sorry, I had looked at this but forgotten to add my reviewed by: Reviewed-by: Alistair Popple On Tuesday, 1 February 2022 10:27:25 AM AEDT Sierra Guiza, Alejandro (Alex) wrote: > Hi Alistair, > This is the last patch to be reviewed from this series. It already has > the changes from > your last feedback (V3). Would you mind to take a look? > Thanks a lot for reviewing the rest! > > Regards, > Alex Sierra > > On 1/28/2022 2:08 PM, Alex Sierra wrote: > > Test cases such as migrate_fault and migrate_multiple, were modified to > > explicit migrate from device to sys memory without the need of page > > faults, when using device coherent type. > > > > Snapshot test case updated to read memory device type first and based > > on that, get the proper returned results migrate_ping_pong test case > > added to test explicit migration from device to sys memory for both > > private and coherent zone types. > > > > Helpers to migrate from device to sys memory and vicerversa > > were also added. > > > > Signed-off-by: Alex Sierra > > Acked-by: Felix Kuehling > > --- > > v2: > > Set FIXTURE_VARIANT to add multiple device types to the FIXTURE. This > > will run all the tests for each device type (private and coherent) in > > case both existed during hmm-test driver probed. > > v4: > > Check for the number of pages successfully migrated from coherent > > device to system at migrate_multiple test. > > --- > > tools/testing/selftests/vm/hmm-tests.c | 123 - > > 1 file changed, 102 insertions(+), 21 deletions(-) > > > > diff --git a/tools/testing/selftests/vm/hmm-tests.c > > b/tools/testing/selftests/vm/hmm-tests.c > > index 203323967b50..84ec8c4a1dc7 100644 > > --- a/tools/testing/selftests/vm/hmm-tests.c > > +++ b/tools/testing/selftests/vm/hmm-tests.c > > @@ -44,6 +44,14 @@ struct hmm_buffer { > > int fd; > > uint64_tcpages; > > uint64_tfaults; > > + int zone_device_type; > > +}; > > + > > +enum { > > + HMM_PRIVATE_DEVICE_ONE, > > + HMM_PRIVATE_DEVICE_TWO, > > + HMM_COHERENCE_DEVICE_ONE, > > + HMM_COHERENCE_DEVICE_TWO, > > }; > > > > #define TWOMEG(1 << 21) > > @@ -60,6 +68,21 @@ FIXTURE(hmm) > > unsigned intpage_shift; > > }; > > > > +FIXTURE_VARIANT(hmm) > > +{ > > + int device_number; > > +}; > > + > > +FIXTURE_VARIANT_ADD(hmm, hmm_device_private) > > +{ > > + .device_number = HMM_PRIVATE_DEVICE_ONE, > > +}; > > + > > +FIXTURE_VARIANT_ADD(hmm, hmm_device_coherent) > > +{ > > + .device_number = HMM_COHERENCE_DEVICE_ONE, > > +}; > > + > > FIXTURE(hmm2) > > { > > int fd0; > > @@ -68,6 +91,24 @@ FIXTURE(hmm2) > > unsigned intpage_shift; > > }; > > > > +FIXTURE_VARIANT(hmm2) > > +{ > > + int device_number0; > > + int device_number1; > > +}; > > + > > +FIXTURE_VARIANT_ADD(hmm2, hmm2_device_private) > > +{ > > + .device_number0 = HMM_PRIVATE_DEVICE_ONE, > > + .device_number1 = HMM_PRIVATE_DEVICE_TWO, > > +}; > > + > > +FIXTURE_VARIANT_ADD(hmm2, hmm2_device_coherent) > > +{ > > + .device_number0 = HMM_COHERENCE_DEVICE_ONE, > > + .device_number1 = HMM_COHERENCE_DEVICE_TWO, > > +}; > > + > > static int hmm_open(int unit) > > { > > char pathname[HMM_PATH_MAX]; > > @@ -81,12 +122,19 @@ static int hmm_open(int unit) > > return fd; > > } > > > > +static bool hmm_is_coherent_type(int dev_num) > > +{ > > + return (dev_num >= HMM_COHERENCE_DEVICE_ONE); > > +} > > + > > FIXTURE_SETUP(hmm) > > { > > self->page_size = sysconf(_SC_PAGE_SIZE); > > self->page_shift = ffs(self->page_size) - 1; > > > > - self->fd = hmm_open(0); > > + self->fd = hmm_open(variant->device_number); > > + if (self->fd < 0 && hmm_is_coherent_type(variant->device_number)) > > + SKIP(exit(0), "DEVICE_COHERENT not available"); > > ASSERT_GE(self->fd, 0); > > } > > > > @@ -95,9 +143,11 @@ FIXTURE_SETUP(hmm2) > > self->page_size = sysconf(_SC_PAGE_SIZE); > > self->page_shift = ffs(self->page_size) - 1; > > > > - self->fd0 = hmm_open(0); > > + self->fd0 = hmm_open(variant->device_number0); > > + if (self->fd0 < 0 && hmm_is_coherent_type(variant->device_number0)) > > + SKIP(exit(0), "DEVICE_COHERENT not available"); > > ASSERT_GE(self->fd0, 0); > > - self->fd1 = hmm_open(1); > > + self->fd1 = hmm_open(variant->device_number1); > > ASSERT_GE(self->fd1, 0); > > } > > > > @@ -144,6 +194,7 @@ static int hmm_dmirror_cmd(int fd, > > } > > buffer->cpages = cmd.cpages; > > buffer->faults = cmd.faults; > > + buffer->zone_device_type = cmd.zone_device_type; > > > > return 0; > > } > > @@ -211,6 +262,20 @@ static void hmm_nanosleep(unsigned int n) > > nanosleep(&t, NULL); > > } > > > > +static int hmm_migrate_sys_to_dev(int fd, > > + struct hmm_buffer *buffer, > > + unsigned long npages) > > +{ > > + r
Re: [Intel-gfx] [PATCH 13/21] fbcon: move more common code into fb_open()
Hi Daniel, I love your patch! Perhaps something to improve: [auto build test WARNING on tegra-drm/drm/tegra/for-next] [also build test WARNING on drm/drm-next linus/master v5.17-rc2 next-20220131] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907 base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220201/202202010613.ht19hztx-...@intel.com/config) compiler: mips-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/7a27c0ce71acfa8b67787d298de9836376fcc323 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907 git checkout 7a27c0ce71acfa8b67787d298de9836376fcc323 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=mips SHELL=/bin/bash drivers/video/fbdev/core/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/video/fbdev/core/fbcon.c: In function 'con2fb_acquire_newinfo': >> drivers/video/fbdev/core/fbcon.c:721:27: warning: variable 'ops' set but not >> used [-Wunused-but-set-variable] 721 | struct fbcon_ops *ops = NULL; | ^~~ vim +/ops +721 drivers/video/fbdev/core/fbcon.c ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 717 ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 718 static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, 7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 719 int unit) ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 720 { ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 @721 struct fbcon_ops *ops = NULL; ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 722 int err; ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 723 ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 724 err = fbcon_open(info); ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 725 if (err) ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 726 return err; ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 727 7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter 2022-01-31 728 ops = info->fbcon_par; d1baa4ffa677bf6 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17 729 d1baa4ffa677bf6 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17 730 if (vc) b73deed32d08740 drivers/video/console/fbcon.cAntonino A. Daplas 2006-01-09 731 set_blitting_type(vc, info); ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 732 ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 733 return err; ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 734 } ^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 735 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[drm-intel:for-linux-next-fixes 6/7] drivers/gpu/drm/i915/i915_vma.c:451:4: error: 'ret' undeclared; did you mean 'net'?
tree: git://anongit.freedesktop.org/drm-intel for-linux-next-fixes head: ea33c6d63f87e34b14dba6f2804990a5fc5a60d7 commit: 2e872d87cbf2cd02dca570ee187cf35567576a70 [6/7] drm/i915: delete shadow "ret" variable config: x86_64-randconfig-a003-20220131 (https://download.01.org/0day-ci/archive/20220201/202202010616.uhgpmfp0-...@intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): git remote add drm-intel git://anongit.freedesktop.org/drm-intel git fetch --no-tags drm-intel for-linux-next-fixes git checkout 2e872d87cbf2cd02dca570ee187cf35567576a70 # save the config file to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind': >> drivers/gpu/drm/i915/i915_vma.c:451:4: error: 'ret' undeclared (first use in >> this function); did you mean 'net'? 451 |ret = i915_gem_object_wait_moving_fence(vma->obj, true); |^~~ |net drivers/gpu/drm/i915/i915_vma.c:451:4: note: each undeclared identifier is reported only once for each function it appears in vim +451 drivers/gpu/drm/i915/i915_vma.c f6c466b84cfa78 Maarten Lankhorst 2021-11-22 376 b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 377 /** b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 378 * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space. b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 379 * @vma: VMA to map b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 380 * @cache_level: mapping cache level b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 381 * @flags: flags like global or local mapping 2850748ef8763a Chris Wilson 2019-10-04 382 * @work: preallocated worker for allocating and binding the PTE b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 383 * b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 384 * DMA addresses are taken from the scatter-gather table of this object (or of b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 385 * this VMA in case of non-default GGTT views) and PTE entries set up. b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 386 * Note that DMA addresses are also the only part of the SG table we care about. b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 387 */ 2850748ef8763a Chris Wilson 2019-10-04 388 int i915_vma_bind(struct i915_vma *vma, 2850748ef8763a Chris Wilson 2019-10-04 389 enum i915_cache_level cache_level, 2850748ef8763a Chris Wilson 2019-10-04 390 u32 flags, 2850748ef8763a Chris Wilson 2019-10-04 391 struct i915_vma_work *work) b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 392 { b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 393 u32 bind_flags; b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 394 u32 vma_flags; b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 395 c2ea703dcafccf Thomas Hellström 2021-12-21 396 lockdep_assert_held(&vma->vm->mutex); aa149431279166 Chris Wilson 2017-02-25 397 GEM_BUG_ON(!drm_mm_node_allocated(&vma->node)); aa149431279166 Chris Wilson 2017-02-25 398 GEM_BUG_ON(vma->size > vma->node.size); aa149431279166 Chris Wilson 2017-02-25 399 bbb8a9d7e000c9 Tvrtko Ursulin 2018-10-12 400 if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start, aa149431279166 Chris Wilson 2017-02-25 401 vma->node.size, aa149431279166 Chris Wilson 2017-02-25 402 vma->vm->total))) aa149431279166 Chris Wilson 2017-02-25 403 return -ENODEV; aa149431279166 Chris Wilson 2017-02-25 404 bbb8a9d7e000c9 Tvrtko Ursulin 2018-10-12 405 if (GEM_DEBUG_WARN_ON(!flags)) b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 406 return -EINVAL; b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 407 2850748ef8763a Chris Wilson 2019-10-04 408 bind_flags = flags; 2850748ef8763a Chris Wilson 2019-10-04 409 bind_flags &= I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND; b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 410 4dd2fbbfb532d0 Chris Wilson 2019-09-11 411 vma_flags = atomic_read(&vma->flags); 4dd2fbbfb532d0 Chris Wilson 2019-09-11 412 vma_flags &= I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND; aedbe0a1af585e Chris Wilson 2020-05-21 413 b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 414 bind_flags &= ~vma_flags; b42fe9ca0a1e2b Joonas Lahtinen2016-11-11 415 if (bind_flags == 0)
Re: [PATCH v5 09/10] tools: update hmm-test to support device coherent type
Hi Alistair, This is the last patch to be reviewed from this series. It already has the changes from your last feedback (V3). Would you mind to take a look? Thanks a lot for reviewing the rest! Regards, Alex Sierra On 1/28/2022 2:08 PM, Alex Sierra wrote: Test cases such as migrate_fault and migrate_multiple, were modified to explicit migrate from device to sys memory without the need of page faults, when using device coherent type. Snapshot test case updated to read memory device type first and based on that, get the proper returned results migrate_ping_pong test case added to test explicit migration from device to sys memory for both private and coherent zone types. Helpers to migrate from device to sys memory and vicerversa were also added. Signed-off-by: Alex Sierra Acked-by: Felix Kuehling --- v2: Set FIXTURE_VARIANT to add multiple device types to the FIXTURE. This will run all the tests for each device type (private and coherent) in case both existed during hmm-test driver probed. v4: Check for the number of pages successfully migrated from coherent device to system at migrate_multiple test. --- tools/testing/selftests/vm/hmm-tests.c | 123 - 1 file changed, 102 insertions(+), 21 deletions(-) diff --git a/tools/testing/selftests/vm/hmm-tests.c b/tools/testing/selftests/vm/hmm-tests.c index 203323967b50..84ec8c4a1dc7 100644 --- a/tools/testing/selftests/vm/hmm-tests.c +++ b/tools/testing/selftests/vm/hmm-tests.c @@ -44,6 +44,14 @@ struct hmm_buffer { int fd; uint64_tcpages; uint64_tfaults; + int zone_device_type; +}; + +enum { + HMM_PRIVATE_DEVICE_ONE, + HMM_PRIVATE_DEVICE_TWO, + HMM_COHERENCE_DEVICE_ONE, + HMM_COHERENCE_DEVICE_TWO, }; #define TWOMEG (1 << 21) @@ -60,6 +68,21 @@ FIXTURE(hmm) unsigned intpage_shift; }; +FIXTURE_VARIANT(hmm) +{ + int device_number; +}; + +FIXTURE_VARIANT_ADD(hmm, hmm_device_private) +{ + .device_number = HMM_PRIVATE_DEVICE_ONE, +}; + +FIXTURE_VARIANT_ADD(hmm, hmm_device_coherent) +{ + .device_number = HMM_COHERENCE_DEVICE_ONE, +}; + FIXTURE(hmm2) { int fd0; @@ -68,6 +91,24 @@ FIXTURE(hmm2) unsigned intpage_shift; }; +FIXTURE_VARIANT(hmm2) +{ + int device_number0; + int device_number1; +}; + +FIXTURE_VARIANT_ADD(hmm2, hmm2_device_private) +{ + .device_number0 = HMM_PRIVATE_DEVICE_ONE, + .device_number1 = HMM_PRIVATE_DEVICE_TWO, +}; + +FIXTURE_VARIANT_ADD(hmm2, hmm2_device_coherent) +{ + .device_number0 = HMM_COHERENCE_DEVICE_ONE, + .device_number1 = HMM_COHERENCE_DEVICE_TWO, +}; + static int hmm_open(int unit) { char pathname[HMM_PATH_MAX]; @@ -81,12 +122,19 @@ static int hmm_open(int unit) return fd; } +static bool hmm_is_coherent_type(int dev_num) +{ + return (dev_num >= HMM_COHERENCE_DEVICE_ONE); +} + FIXTURE_SETUP(hmm) { self->page_size = sysconf(_SC_PAGE_SIZE); self->page_shift = ffs(self->page_size) - 1; - self->fd = hmm_open(0); + self->fd = hmm_open(variant->device_number); + if (self->fd < 0 && hmm_is_coherent_type(variant->device_number)) + SKIP(exit(0), "DEVICE_COHERENT not available"); ASSERT_GE(self->fd, 0); } @@ -95,9 +143,11 @@ FIXTURE_SETUP(hmm2) self->page_size = sysconf(_SC_PAGE_SIZE); self->page_shift = ffs(self->page_size) - 1; - self->fd0 = hmm_open(0); + self->fd0 = hmm_open(variant->device_number0); + if (self->fd0 < 0 && hmm_is_coherent_type(variant->device_number0)) + SKIP(exit(0), "DEVICE_COHERENT not available"); ASSERT_GE(self->fd0, 0); - self->fd1 = hmm_open(1); + self->fd1 = hmm_open(variant->device_number1); ASSERT_GE(self->fd1, 0); } @@ -144,6 +194,7 @@ static int hmm_dmirror_cmd(int fd, } buffer->cpages = cmd.cpages; buffer->faults = cmd.faults; + buffer->zone_device_type = cmd.zone_device_type; return 0; } @@ -211,6 +262,20 @@ static void hmm_nanosleep(unsigned int n) nanosleep(&t, NULL); } +static int hmm_migrate_sys_to_dev(int fd, + struct hmm_buffer *buffer, + unsigned long npages) +{ + return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE_TO_DEV, buffer, npages); +} + +static int hmm_migrate_dev_to_sys(int fd, + struct hmm_buffer *buffer, + unsigned long npages) +{ + return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE_TO_SYS, buffer, npages); +} + /* * Simple NULL test of device open/close. */ @@ -875,7 +940,7 @@ TEST_F(hmm, migrate) ptr[i] = i; /* Migrate memory to device. */ - ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages); + ret = hmm_migrate_sys_t
Re: [PATCH 1/4] drm: Add I2C connector type
On 1/31/22 21:52, Sam Ravnborg wrote: > On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: >> There isn't a connector type for display controllers accesed through I2C, >> most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. >> >> Add an I2C connector type to match the actual connector. >> >> As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector >> type"), user-space should be able to cope with a connector type that does >> not yet understand. >> >> Tested with `modetest -M ssd1307 -c` and shows the connector as unknown-1. > I had expected unknown-21?? > As you pointed out in patch 3/4, I forgot to change DRM_MODE_CONNECTOR_Unknown to DRM_MODE_CONNECTOR_I2C after adding this patch... >> >> Signed-off-by: Javier Martinez Canillas > Reviewed-by: Sam Ravnborg >> --- Thanks! Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 4/4] MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver
Hello Sam, Thanks a lot for your feedback. On 1/31/22 21:46, Sam Ravnborg wrote: > Hi Javier, > > On Mon, Jan 31, 2022 at 09:15:37PM +0100, Javier Martinez Canillas wrote: >> To make sure that tools like the get_maintainer.pl script will suggest >> to Cc me if patches are posted for this driver. >> >> Also include the Device Tree binding for the old ssd1307fb fbdev driver >> since the new DRM driver was made compatible with the existing binding. > > To avoid any confusion add yourself as Maintainer in the > solomon,ssd1307fb.yaml file too. > Agreed. You mentioned in another email though to diverge from the existing DT binding for ssd1307fb. If we decide to keep the backward compatibility then I'll add another patch to the set to list myself as a co-maintainer. > With that done: > Acked-by: Sam Ravnborg > Thanks! Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hello Simon, Thanks for your feedback. On 1/31/22 21:39, Simon Ser wrote: > On Monday, January 31st, 2022 at 21:36, Simon Ser wrote: > >> This driver only advertises XRGB in ssd1307_formats. It would be nice to >> expose R8 as well so that user-space can directly produce suitable buffers. >> It would also be nice to have some kind of preferred format, so that >> user-space >> knows R8 is preferred over XRGB. > > Hm, since the format used by the hw is actually R1, adding that to > drm_fourcc.h > would be even better. > Yes, agreed that would be nice. We discussed this already with Thomas and my suggestion was to land the driver as is, advertising XRGB. Which is also what the other driver using monochrome does (drivers/gpu/drm/tiny/repaper.c): https://www.spinics.net/lists/dri-devel/msg331328.html As a follow-up we can wire up al the needed bits to have a DRM/KMS driver that could expose a R1 format. > Let me know if you want me to type up any of the user-space bits. > Thanks! I also could help to add the needed support in the user-space stack. Best reagards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] mm: add device coherent vma selection for memory migration
Thanks for fixing. I'm guessing Andrew will want you to resend this as part of a new v6 series, but please add: Reviewed-by: Alistair Popple On Tuesday, 1 February 2022 6:48:13 AM AEDT Alex Sierra wrote: > This case is used to migrate pages from device memory, back to system > memory. Device coherent type memory is cache coherent from device and CPU > point of view. > > Signed-off-by: Alex Sierra > Acked-by: Felix Kuehling > --- > v2: > condition added when migrations from device coherent pages. > --- > include/linux/migrate.h | 1 + > mm/migrate.c| 12 +--- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/include/linux/migrate.h b/include/linux/migrate.h > index db96e10eb8da..66a34eae8cb6 100644 > --- a/include/linux/migrate.h > +++ b/include/linux/migrate.h > @@ -130,6 +130,7 @@ static inline unsigned long migrate_pfn(unsigned long pfn) > enum migrate_vma_direction { > MIGRATE_VMA_SELECT_SYSTEM = 1 << 0, > MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1, > + MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2, > }; > > struct migrate_vma { > diff --git a/mm/migrate.c b/mm/migrate.c > index cd137aedcfe5..69c6830c47c6 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -2264,15 +2264,21 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, > if (is_writable_device_private_entry(entry)) > mpfn |= MIGRATE_PFN_WRITE; > } else { > - if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) > - goto next; > pfn = pte_pfn(pte); > - if (is_zero_pfn(pfn)) { > + if (is_zero_pfn(pfn) && > + (migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) { > mpfn = MIGRATE_PFN_MIGRATE; > migrate->cpages++; > goto next; > } > page = vm_normal_page(migrate->vma, addr, pte); > + if (page && !is_zone_device_page(page) && > + !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) > + goto next; > + else if (page && is_device_coherent_page(page) && > + (!(migrate->flags & > MIGRATE_VMA_SELECT_DEVICE_COHERENT) || > + page->pgmap->owner != migrate->pgmap_owner)) > + goto next; > mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE; > mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0; > } >
Re: [PATCH v2 4/4] drm/msm/adreno: Update the name of 7c3 gpu
On Wed 19 Jan 09:21 CST 2022, Akhil P Oommen wrote: > Update the name in the gpulist for 7c3 gpu as per the latest > recommendation. > I was skeptical when this was introduced and you proved my point. Give it a name based on the Adreno revision or possibly the part number and leave it at that. Regards, Bjorn > Signed-off-by: Akhil P Oommen > --- > > (no changes since v1) > > drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c > b/drivers/gpu/drm/msm/adreno/adreno_device.c > index 946f505..bd4d6a1 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > @@ -330,7 +330,7 @@ static const struct adreno_info gpulist[] = { > .hwcg = a660_hwcg, > }, { > .rev = ADRENO_REV(6, 3, 5, ANY_ID), > - .name = "Adreno 7c Gen 3", > + .name = "Adreno 7c+ Gen 3", > .fw = { > [ADRENO_FW_SQE] = "a660_sqe.fw", > [ADRENO_FW_GMU] = "a660_gmu.bin", > -- > 2.7.4 >
Re: [PATCH v2 2/4] arm64: dts: qcom: sc7280: Support gpu speedbin
On Wed 19 Jan 09:21 CST 2022, Akhil P Oommen wrote: > Add the speedbin fuse and the required opps to support gpu sku. > > Signed-off-by: Akhil P Oommen > --- > > (no changes since v1) > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 46 > > 1 file changed, 46 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 365a2e0..f8fc8b8 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -605,6 +605,11 @@ > power-domains = <&rpmhpd SC7280_MX>; > #address-cells = <1>; > #size-cells = <1>; > + > + gpu_speed_bin: gpu_speed_bin@1e9 { No underscores in node names please. Regards, Bjorn > + reg = <0x1e9 0x2>; > + bits = <5 8>; > + }; > }; > > sdhc_1: sdhci@7c4000 { > @@ -1762,6 +1767,9 @@ > interconnect-names = "gfx-mem"; > #cooling-cells = <2>; > > + nvmem-cells = <&gpu_speed_bin>; > + nvmem-cell-names = "speed_bin"; > + > gpu_opp_table: opp-table { > compatible = "operating-points-v2"; > > @@ -1769,18 +1777,56 @@ > opp-hz = /bits/ 64 <31500>; > opp-level = > ; > opp-peak-kBps = <1804000>; > + opp-supported-hw = <0x03>; > }; > > opp-45000 { > opp-hz = /bits/ 64 <45000>; > opp-level = ; > opp-peak-kBps = <4068000>; > + opp-supported-hw = <0x03>; > }; > > opp-55000 { > opp-hz = /bits/ 64 <55000>; > opp-level = > ; > opp-peak-kBps = <6832000>; > + opp-supported-hw = <0x03>; > + }; > + > + opp-60800 { > + opp-hz = /bits/ 64 <60800>; > + opp-level = > ; > + opp-peak-kBps = <8368000>; > + opp-supported-hw = <0x02>; > + }; > + > + opp-7 { > + opp-hz = /bits/ 64 <7>; > + opp-level = ; > + opp-peak-kBps = <8532000>; > + opp-supported-hw = <0x02>; > + }; > + > + opp-81200 { > + opp-hz = /bits/ 64 <81200>; > + opp-level = > ; > + opp-peak-kBps = <8532000>; > + opp-supported-hw = <0x02>; > + }; > + > + opp-84000 { > + opp-hz = /bits/ 64 <84000>; > + opp-level = > ; > + opp-peak-kBps = <8532000>; > + opp-supported-hw = <0x02>; > + }; > + > + opp-9 { > + opp-hz = /bits/ 64 <9>; > + opp-level = > ; > + opp-peak-kBps = <8532000>; > + opp-supported-hw = <0x02>; > }; > }; > }; > -- > 2.7.4 >
Re: [PATCH v2 1/4] drm/msm/adreno: Add support for Adreno 8c Gen 3
On Wed 19 Jan 09:21 CST 2022, Akhil P Oommen wrote: > Add support for "Adreno 8c Gen 3" gpu along with the necessary speedbin > support. > > Signed-off-by: Akhil P Oommen > --- > > Changes in v2: > - Fix a bug in adreno_cmp_rev() > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 21 ++ > drivers/gpu/drm/msm/adreno/adreno_device.c | 34 > +++--- > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 10 +++-- > 3 files changed, 56 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 51b8377..9268ce3 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -10,7 +10,6 @@ > > #include > #include > -#include > #include > > #define GPU_PAS_ID 13 > @@ -1734,6 +1733,18 @@ static u32 a618_get_speed_bin(u32 fuse) > return UINT_MAX; > } > > +static u32 adreno_7c3_get_speed_bin(u32 fuse) > +{ > + if (fuse == 0) > + return 0; > + else if (fuse == 117) > + return 0; > + else if (fuse == 190) Depending on your answer below this isn't an adreno_7c3 speed bin. I think you should name this function adreno_635_get_speed_bin(). > + return 1; > + > + return UINT_MAX; > +} > + > static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32 > fuse) > { > u32 val = UINT_MAX; > @@ -1741,6 +1752,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct > adreno_rev rev, u32 fuse) > if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev)) > val = a618_get_speed_bin(fuse); > > + if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev)) > + val = adreno_7c3_get_speed_bin(fuse); > + > if (val == UINT_MAX) { > DRM_DEV_ERROR(dev, > "missing support for speed-bin: %u. Some OPPs may not > be supported by hardware", > @@ -1753,11 +1767,10 @@ static u32 fuse_to_supp_hw(struct device *dev, struct > adreno_rev rev, u32 fuse) > > static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev) > { > - u32 supp_hw = UINT_MAX; > - u32 speedbin; > + u32 speedbin, supp_hw = UINT_MAX; > int ret; > > - ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", &speedbin); > + ret = adreno_read_speedbin(dev, &speedbin); > /* >* -ENOENT means that the platform doesn't support speedbin which is >* fine > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c > b/drivers/gpu/drm/msm/adreno/adreno_device.c > index 9300583..946f505 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > @@ -6,6 +6,7 @@ > * Copyright (c) 2014,2017 The Linux Foundation. All rights reserved. > */ > > +#include > #include "adreno_gpu.h" > > bool hang_debug = false; > @@ -317,6 +318,17 @@ static const struct adreno_info gpulist[] = { > .zapfw = "a660_zap.mdt", > .hwcg = a660_hwcg, > }, { > + .rev = ADRENO_REV_SKU(6, 3, 5, ANY_ID, 190), So Adreno 7c Gen 3 revision 190 is actually a 8c Gen 3? Why can't we just keep the marketing guys out the kernel and name things based on what they really are!? Regards, Bjorn > + .name = "Adreno 8c Gen 3", > + .fw = { > + [ADRENO_FW_SQE] = "a660_sqe.fw", > + [ADRENO_FW_GMU] = "a660_gmu.bin", > + }, > + .gmem = SZ_512K, > + .inactive_period = DRM_MSM_INACTIVE_PERIOD, > + .init = a6xx_gpu_init, > + .hwcg = a660_hwcg, > + }, { > .rev = ADRENO_REV(6, 3, 5, ANY_ID), > .name = "Adreno 7c Gen 3", > .fw = { > @@ -365,13 +377,19 @@ static inline bool _rev_match(uint8_t entry, uint8_t id) > return (entry == ANY_ID) || (entry == id); > } > > +static inline bool _rev_match_sku(uint16_t entry, uint16_t id) > +{ > + return (entry == ANY_SKU) || (entry == id); > +} > + > bool adreno_cmp_rev(struct adreno_rev rev1, struct adreno_rev rev2) > { > > return _rev_match(rev1.core, rev2.core) && > _rev_match(rev1.major, rev2.major) && > _rev_match(rev1.minor, rev2.minor) && > - _rev_match(rev1.patchid, rev2.patchid); > + _rev_match(rev1.patchid, rev2.patchid) && > + _rev_match_sku(rev1.sku, rev2.sku); > } > > const struct adreno_info *adreno_info(struct adreno_rev rev) > @@ -445,12 +463,17 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev) > return gpu; > } > > +int adreno_read_speedbin(struct device *dev, u32 *speedbin) > +{ > + return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin); > +} > + > static int find_chipid(struct device *dev, struct adreno_rev *rev) > { > struct device_node *node = dev->of_node; > const char *compat; > int ret; > -
linux-next: build failure after merge of the drm-intel-fixes tree
Hi all, After merging the drm-intel-fixes tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind': drivers/gpu/drm/i915/i915_vma.c:451:25: error: 'ret' undeclared (first use in this function); did you mean 'net'? 451 | ret = i915_gem_object_wait_moving_fence(vma->obj, true); | ^~~ | net Caused by commit 2e872d87cbf2 ("drm/i915: delete shadow "ret" variable") I have reverted that commit for today. -- Cheers, Stephen Rothwell pgpLMH9X4kdm5.pgp Description: OpenPGP digital signature
Re: [PATCH] drm/msm/dp: add wide bus support
On 1/28/2022 8:59 PM, Dmitry Baryshkov wrote: Hi, Thank you for your patch. On Fri, 28 Jan 2022 at 20:29, Kuogee Hsieh wrote: Normally, mdp will push one pixel of data per pixel clock to interface to display. Wide bus feature will increase bus width from 32 bits to 64 bits so that it can push two pixel of data per pixel clock to interface to display. This feature is pre requirement to support 4k resolution. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 16 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 108 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 5 + drivers/gpu/drm/msm/dp/dp_catalog.c| 11 ++- drivers/gpu/drm/msm/dp/dp_catalog.h| 1 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +- drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c| 17 drivers/gpu/drm/msm/dp/dp_display.h| 3 + drivers/gpu/drm/msm/dp/dp_parser.c | 26 + drivers/gpu/drm/msm/dp/dp_parser.h | 2 + drivers/gpu/drm/msm/msm_drv.h | 9 ++ Can we get this split into logical chunks please? 14 files changed, 190 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1e648db..e2fb5bc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -199,6 +199,8 @@ struct dpu_encoder_virt { struct msm_display_info disp_info; + struct msm_op_info op_info; + bool idle_pc_supported; struct mutex rc_lock; enum dpu_enc_rc_states rc_state; @@ -217,6 +219,13 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc) +{ + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); + + return dpu_enc->op_info.wide_bus_en; +} + static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned bpc) { struct dpu_hw_dither_cfg dither_cfg = { 0 }; @@ -2112,6 +2121,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + struct msm_op_info *op_info; int ret = 0; dpu_enc = to_dpu_encoder_virt(enc); @@ -2128,8 +2138,12 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, timer_setup(&dpu_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) + else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) { dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]]; + op_info = &priv->op_info[disp_info->h_tile_instance[0]]; op_info should be defined per INTF rather than per h_tile. This way you won't have to check for intf_type here. The more i am thinking, the more i think op_info should be along with controller. widebus belongs to mdp fetching process and intf only care of raw bit stream. it has nothing to do with intf. The other problem is we can not access inside of prov->dp here. + dpu_enc->op_info = *op_info; So... we set this data in msm_drm_private only to copy it to dpu_encoder_virt? Please don't do this. Allow one to query the data from the DP rather than blindly copying it over and over again. Is this only copy one time? maybe change to pointer work better? + + } INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index e241914..0d73550 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc); */ int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc); +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index ddd9d89..04ac2dc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params( timing->v_back_porch += timing->v_front_porch; timing->v_
Re: [PATCH v8 00/10] vgaarb: Rework default VGA device selection
[+to Maarten, Maxime, Thomas; beginning of thread: https://lore.kernel.org/r/20220106000658.243509-1-helg...@kernel.org] On Wed, Jan 05, 2022 at 06:06:48PM -0600, Bjorn Helgaas wrote: > From: Bjorn Helgaas > > Current default VGA device selection fails in some cases because part of it > is done in the vga_arb_device_init() subsys_initcall, and some arches > enumerate PCI devices in pcibios_init(), which runs *after* that. Where are we at with this series? Is there anything I can do to move it forward? Bjorn > For example: > > - On BMC system, the AST2500 bridge [1a03:1150] does not implement > PCI_BRIDGE_CTL_VGA. This is perfectly legal but means the legacy VGA > resources won't reach downstream devices unless they're included in the > usual bridge windows. > > - vga_arb_select_default_device() will set a device below such a bridge > as the default VGA device as long as it has PCI_COMMAND_IO and > PCI_COMMAND_MEMORY enabled. > > - vga_arbiter_add_pci_device() is called for every VGA device, either at > boot-time or at hot-add time, and it will also set the device as the > default VGA device, but ONLY if all bridges leading to it implement > PCI_BRIDGE_CTL_VGA. > > - This difference between vga_arb_select_default_device() and > vga_arbiter_add_pci_device() means that a device below an AST2500 or > similar bridge can only be set as the default if it is enumerated > before vga_arb_device_init(). > > - On ACPI-based systems, PCI devices are enumerated by acpi_init(), which > runs before vga_arb_device_init(). > > - On non-ACPI systems, like on MIPS system, they are enumerated by > pcibios_init(), which typically runs *after* vga_arb_device_init(). > > This series consolidates all the default VGA device selection in > vga_arbiter_add_pci_device(), which is always called after enumerating a > PCI device. > > Almost all the work here is Huacai's. I restructured it a little bit and > added a few trivial patches on top. > > I'd like to move vgaarb.c to drivers/pci eventually, but there's another > initcall ordering snag that needs to be resolved first, so this leaves > it where it is. > > Bjorn > > Version history: > V0 original implementation as final quirk to set default device. > https://lore.kernel.org/r/20210514080025.1828197-6-chenhua...@loongson.cn > > V1 rework vgaarb to do all default device selection in > vga_arbiter_add_pci_device(). > https://lore.kernel.org/r/20210705100503.1120643-1-chenhua...@loongson.cn > > V2 move arbiter to PCI subsystem, fix nits. > https://lore.kernel.org/r/20210722212920.347118-1-helg...@kernel.org > > V3 rewrite the commit log of the last patch (which is also summarized > by Bjorn). > https://lore.kernel.org/r/20210820100832.663931-1-chenhua...@loongson.cn > > V4 split the last patch to two steps. > https://lore.kernel.org/r/20210827083129.2781420-1-chenhua...@loongson.cn > > V5 split Patch-9 again and sort the patches. > https://lore.kernel.org/r/20210911093056.1555274-1-chenhua...@loongson.cn > > V6 split Patch-5 again and sort the patches again. > https://lore.kernel.org/r/20210916082941.3421838-1-chenhua...@loongson.cn > > V7 stop moving vgaarb to drivers/pci because of ordering issues with > misc_init(). > https://lore.kernel.org/r/20211015061512.2941859-1-chenhua...@loongson.cn > https://lore.kernel.org/r/caahv-h7fhajm-ha42z1dlre4pvc9frfyeu27khwcywkkmft...@mail.gmail.com > > > Bjorn Helgaas (8): > vgaarb: Factor out vga_select_framebuffer_device() > vgaarb: Factor out default VGA device selection > vgaarb: Move framebuffer detection to ADD_DEVICE path > vgaarb: Move non-legacy VGA detection to ADD_DEVICE path > vgaarb: Move disabled VGA device detection to ADD_DEVICE path > vgaarb: Remove empty vga_arb_device_card_gone() > vgaarb: Use unsigned format string to print lock counts > vgaarb: Replace full MIT license text with SPDX identifier > > Huacai Chen (2): > vgaarb: Move vga_arb_integrated_gpu() earlier in file > vgaarb: Log bridge control messages when adding devices > > drivers/gpu/vga/vgaarb.c | 311 +++ > 1 file changed, 154 insertions(+), 157 deletions(-) > > -- > 2.25.1 >
Re: [PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Mon, Jan 31, 2022 at 09:29:16PM +0100, Javier Martinez Canillas wrote: > Add a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED > controllers that can be programmed via an I2C interface. This is a port > of the ssd1307fb driver that already supports these devices. > > A Device Tree binding is not added because the DRM driver is compatible > with the existing binding for the ssd1307fb driver. The driver uses the pwms property for the backlight. It would have been much better to bite the bullet and require the backlight to be specified using a backlight node in the DT. This is the standard way to do it and then the driver could use the existing backlight driver rather than embedding a backlight driver here. It would cost some backward compatibility - but looking forward this is the right thing to do. > > Signed-off-by: Javier Martinez Canillas Some random comments in the following. Sam > --- > > drivers/gpu/drm/tiny/Kconfig | 12 + > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/tiny/ssd1307.c | 976 + > 3 files changed, 989 insertions(+) > create mode 100644 drivers/gpu/drm/tiny/ssd1307.c > > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 712e0004e96e..25c9c013bcda 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -157,6 +157,18 @@ config TINYDRM_REPAPER > > If M is selected the module will be called repaper. > > +config TINYDRM_SSD1307 > + tristate "DRM support for Solomon SSD1307 OLED displays" Use SSD130X here - so SSD1306 users can find it. > + depends on DRM && I2C > + select DRM_KMS_HELPER > + select DRM_GEM_SHMEM_HELPER > + select BACKLIGHT_CLASS_DEVICE > + help > + DRM driver for the SSD1305, SSD1306, SSD1307 and SSD1309 Solomon > + OLED controllers that can be programmed via an I2C interface. > + > + If M is selected the module will be called ssd1307. > + > config TINYDRM_ST7586 > tristate "DRM support for Sitronix ST7586 display panels" > depends on DRM && SPI > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > index 5d5505d40e7b..38c4ce96 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -12,5 +12,6 @@ obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o > obj-$(CONFIG_TINYDRM_ILI9486)+= ili9486.o > obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o > obj-$(CONFIG_TINYDRM_REPAPER)+= repaper.o > +obj-$(CONFIG_TINYDRM_SSD1307)+= ssd1307.o > obj-$(CONFIG_TINYDRM_ST7586) += st7586.o > obj-$(CONFIG_TINYDRM_ST7735R)+= st7735r.o > diff --git a/drivers/gpu/drm/tiny/ssd1307.c b/drivers/gpu/drm/tiny/ssd1307.c > new file mode 100644 > index ..4ea223674587 > --- /dev/null > +++ b/drivers/gpu/drm/tiny/ssd1307.c > @@ -0,0 +1,976 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * DRM driver for Solomon SSD1307 OLED displays > + * > + * Copyright 2022 Red Hat Inc. > + * > + * Based on drivers/video/fbdev/ssd1307fb.c > + * Copyright 2012 Free Electrons > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRIVER_NAME "ssd1307" > +#define DRIVER_DESC "DRM driver for Solomon SSD1307 OLED displays" > +#define DRIVER_DATE "20220131" > +#define DRIVER_MAJOR 1 > +#define DRIVER_MINOR 0 > + > +#define SSD1307_DATA 0x40 > +#define SSD1307_COMMAND 0x80 > + > +#define SSD1307_SET_ADDRESS_MODE 0x20 > +#define SSD1307_SET_ADDRESS_MODE_HORIZONTAL (0x00) > +#define SSD1307_SET_ADDRESS_MODE_VERTICAL(0x01) > +#define SSD1307_SET_ADDRESS_MODE_PAGE(0x02) > +#define SSD1307_SET_COL_RANGE0x21 > +#define SSD1307_SET_PAGE_RANGE 0x22 > +#define SSD1307_CONTRAST 0x81 > +#define SSD1307_SET_LOOKUP_TABLE 0x91 > +#define SSD1307_CHARGE_PUMP 0x8d > +#define SSD1307_SEG_REMAP_ON 0xa1 > +#define SSD1307_DISPLAY_OFF 0xae > +#define SSD1307_SET_MULTIPLEX_RATIO 0xa8 > +#define SSD1307_DISPLAY_ON 0xaf > +#define SSD1307_START_PAGE_ADDRE
[PATCH 18/21] fbcon: untangle fbcon_exit
There's a bunch of confusions going on here: - The deferred fbcon setup notifier should only be cleaned up from fb_console_exit(), to be symmetric with fb_console_init() - We also need to make sure we don't race with the work, which means temporarily dropping the console lock (or we can deadlock) - That also means no point in clearing deferred_takeover, we are unloading everything anyway. - Finally rename fbcon_exit to fbcon_release_all and move it, since that's what's it doing when being called from consw->con_deinit through fbcon_deinit. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Claudio Suarez Cc: Du Cheng Cc: Tetsuo Handa --- drivers/video/fbdev/core/fbcon.c | 63 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 5c14e24d14a1..22581952b4fd 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -185,7 +185,6 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var, int unit); static void fbcon_modechanged(struct fb_info *info); static void fbcon_set_all_vcs(struct fb_info *info); -static void fbcon_exit(void); static struct device *fbcon_device; @@ -1149,6 +1148,27 @@ static void fbcon_free_font(struct fbcon_display *p, bool freefont) static void set_vc_hi_font(struct vc_data *vc, bool set); +static void fbcon_release_all(void) +{ + struct fb_info *info; + int i, j, mapped; + + for_each_registered_fb(i) { + mapped = 0; + info = registered_fb[i]; + + for (j = first_fb_vc; j <= last_fb_vc; j++) { + if (con2fb_map[j] == i) { + mapped = 1; + con2fb_map[j] = -1; + } + } + + if (mapped) + fbcon_release(info); + } +} + static void fbcon_deinit(struct vc_data *vc) { struct fbcon_display *p = &fb_display[vc->vc_num]; @@ -1188,7 +1208,7 @@ static void fbcon_deinit(struct vc_data *vc) set_vc_hi_font(vc, false); if (!con_is_bound(&fb_con)) - fbcon_exit(); + fbcon_release_all(); if (vc->vc_num == logo_shown) logo_shown = FBCON_LOGO_CANSHOW; @@ -3316,34 +3336,6 @@ static void fbcon_start(void) #endif } -static void fbcon_exit(void) -{ - struct fb_info *info; - int i, j, mapped; - -#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER - if (deferred_takeover) { - dummycon_unregister_output_notifier(&fbcon_output_nb); - deferred_takeover = false; - } -#endif - - for_each_registered_fb(i) { - mapped = 0; - info = registered_fb[i]; - - for (j = first_fb_vc; j <= last_fb_vc; j++) { - if (con2fb_map[j] == i) { - mapped = 1; - con2fb_map[j] = -1; - } - } - - if (mapped) - fbcon_release(info); - } -} - void __init fb_console_init(void) { int i; @@ -3383,10 +3375,19 @@ static void __exit fbcon_deinit_device(void) void __exit fb_console_exit(void) { +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER + console_lock(); + if (deferred_takeover) + dummycon_unregister_output_notifier(&fbcon_output_nb); + console_unlock(); + + cancel_work_sync(&fbcon_deferred_takeover_work); +#endif + console_lock(); fbcon_deinit_device(); device_destroy(fb_class, MKDEV(0, 0)); - fbcon_exit(); + do_unregister_con_driver(&fb_con); console_unlock(); } -- 2.33.0
[PATCH 21/21] fbdev: Make registered_fb[] private to fbmem.c
Well except when the olpc dcon fbdev driver is enabled, that thing digs around in there in rather unfixable ways. Cc oldc_dcon maintainers as fyi. Cc: Jens Frederich Cc: Jon Nettleton Cc: Greg Kroah-Hartman Cc: linux-stag...@lists.linux.dev Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Helge Deller Cc: Matthew Wilcox Cc: Sam Ravnborg Cc: Tetsuo Handa Cc: Zhen Lei Cc: Alex Deucher Cc: Xiyu Yang Cc: linux-fb...@vger.kernel.org Cc: Zheyu Ma Cc: Guenter Roeck --- drivers/video/fbdev/core/fbmem.c | 8 ++-- include/linux/fb.h | 7 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 904ef1250677..dad6572942fa 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -48,10 +48,14 @@ static DEFINE_MUTEX(registration_lock); struct fb_info *registered_fb[FB_MAX] __read_mostly; -EXPORT_SYMBOL(registered_fb); - int num_registered_fb __read_mostly; +#if IS_ENABLED(CONFIG_OLPC_DCON) +EXPORT_SYMBOL(registered_fb); EXPORT_SYMBOL(num_registered_fb); +#endif +#define for_each_registered_fb(i) \ + for (i = 0; i < FB_MAX; i++)\ + if (!registered_fb[i]) {} else bool fb_center_logo __read_mostly; diff --git a/include/linux/fb.h b/include/linux/fb.h index a8a00d2ba1f3..e236817502c2 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -622,16 +622,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var, extern int fb_get_options(const char *name, char **option); extern int fb_new_modelist(struct fb_info *info); +#if IS_ENABLED(CONFIG_OLPC_DCON) extern struct fb_info *registered_fb[FB_MAX]; + extern int num_registered_fb; +#endif extern bool fb_center_logo; extern int fb_logo_count; extern struct class *fb_class; -#define for_each_registered_fb(i) \ - for (i = 0; i < FB_MAX; i++)\ - if (!registered_fb[i]) {} else - static inline void lock_fb_info(struct fb_info *info) { mutex_lock(&info->lock); -- 2.33.0
[PATCH 19/21] fbcon: Maintain a private array of fb_info
Accessing the one in fbmem.c without taking the right locks is a bad idea. Instead maintain our own private copy, which is fully protected by console_lock() (like everything else in fbcon.c). That copy is serialized through fbcon_fb_registered/unregistered() calls. Also this means we do not need to hold a full fb_info reference, which is nice because doing so would mean a refcount loop between the console and the fb_info. But it's also not nice since it means console_lock() must be held absolutely everywhere. Well strictly speaking we could still try to do some refcounting games again by calling get_fb_info before we drop the console_lock. But things will get tricky. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Tetsuo Handa Cc: Claudio Suarez Cc: Du Cheng Cc: Greg Kroah-Hartman --- drivers/video/fbdev/core/fbcon.c | 82 +--- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 22581952b4fd..a0ca34b29615 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -86,10 +86,6 @@ * - fbcon state itself is protected by the console_lock, and the code does a * pretty good job at making sure that lock is held everywhere it's needed. * - * - access to the registered_fb array is entirely unprotected. This should use - * proper object lifetime handling, i.e. get/put_fb_info. This also means - * switching from indices to proper pointers for fb_info everywhere. - * * - fbcon doesn't bother with fb_lock/unlock at all. This is buggy, since it * means concurrent access to the same fbdev from both fbcon and userspace * will blow up. To fix this all fbcon calls from fbmem.c need to be moved out @@ -107,6 +103,13 @@ enum { static struct fbcon_display fb_display[MAX_NR_CONSOLES]; +struct fb_info *fbcon_registered_fb[FB_MAX]; +int fbcon_num_registered_fb; + +#define fbcon_for_each_registered_fb(i)\ + for (i = 0; WARN_CONSOLE_UNLOCKED(), i < FB_MAX; i++) \ + if (!fbcon_registered_fb[i]) {} else + static signed char con2fb_map[MAX_NR_CONSOLES]; static signed char con2fb_map_boot[MAX_NR_CONSOLES]; @@ -114,12 +117,7 @@ static struct fb_info *fbcon_info_from_console(int console) { WARN_CONSOLE_UNLOCKED(); - /* -* Note that only con2fb_map is protected by the console lock, -* registered_fb is protected by a separate mutex. This lookup can -* therefore race. -*/ - return registered_fb[con2fb_map[console]]; + return fbcon_registered_fb[con2fb_map[console]]; } static int logo_lines; @@ -516,7 +514,7 @@ static int do_fbcon_takeover(int show_logo) { int err, i; - if (!num_registered_fb) + if (!fbcon_num_registered_fb) return -ENODEV; if (!show_logo) @@ -822,7 +820,7 @@ static int set_con2fb_map(int unit, int newidx, int user) { struct vc_data *vc = vc_cons[unit].d; int oldidx = con2fb_map[unit]; - struct fb_info *info = registered_fb[newidx]; + struct fb_info *info = fbcon_registered_fb[newidx]; struct fb_info *oldinfo = NULL; int found, err = 0, show_logo; @@ -840,7 +838,7 @@ static int set_con2fb_map(int unit, int newidx, int user) } if (oldidx != -1) - oldinfo = registered_fb[oldidx]; + oldinfo = fbcon_registered_fb[oldidx]; found = search_fb_in_map(newidx); @@ -932,13 +930,13 @@ static const char *fbcon_startup(void) * If num_registered_fb is zero, this is a call for the dummy part. * The frame buffer devices weren't initialized yet. */ - if (!num_registered_fb || info_idx == -1) + if (!fbcon_num_registered_fb || info_idx == -1) return display_desc; /* * Instead of blindly using registered_fb[0], we use info_idx, set by * fbcon_fb_registered(); */ - info = registered_fb[info_idx]; + info = fbcon_registered_fb[info_idx]; if (!info) return NULL; @@ -1153,9 +1151,9 @@ static void fbcon_release_all(void) struct fb_info *info; int i, j, mapped; - for_each_registered_fb(i) { + fbcon_for_each_registered_fb(i) { mapped = 0; - info = registered_fb[i]; + info = fbcon_registered_fb[i]; for (j = first_fb_vc; j <= last_fb_vc; j++) { if (con2fb_map[j] == i) { @@ -1182,7 +1180,7 @@ static void fbcon_deinit(struct vc_data *vc) if (idx == -1) goto finished; - info = registered_fb[idx]; + info = fbcon_registered_fb[idx]; if (!info) goto finished; @@ -2118,9 +2116,9 @@ static int fbcon_switch(struct vc_data *vc) * * info->currcon = vc->vc_num; */ - for_each_regis
[PATCH 20/21] Revert "fbdev: Prevent probing generic drivers if a FB is already registered"
This reverts commit fb561bf9abde49f7e00fdbf9ed2ccf2d86cac8ee. With commit 27599aacbaefcbf2af7b06b0029459bbf682000d Author: Thomas Zimmermann Date: Tue Jan 25 10:12:18 2022 +0100 fbdev: Hot-unplug firmware fb devices on forced removal this should be fixed properly and we can remove this somewhat hackish check here (e.g. this won't catch drm drivers if fbdev emulation isn't enabled). Cc: Thomas Zimmermann Cc: Zack Rusin Cc: Javier Martinez Canillas Cc: Zack Rusin Cc: Hans de Goede Cc: Ilya Trukhanov Signed-off-by: Daniel Vetter Cc: Peter Jones Cc: linux-fb...@vger.kernel.org --- drivers/video/fbdev/efifb.c| 11 --- drivers/video/fbdev/simplefb.c | 11 --- 2 files changed, 22 deletions(-) diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index ea42ba6445b2..edca3703b964 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -351,17 +351,6 @@ static int efifb_probe(struct platform_device *dev) char *option = NULL; efi_memory_desc_t md; - /* -* Generic drivers must not be registered if a framebuffer exists. -* If a native driver was probed, the display hardware was already -* taken and attempting to use the system framebuffer is dangerous. -*/ - if (num_registered_fb > 0) { - dev_err(&dev->dev, - "efifb: a framebuffer is already registered\n"); - return -EINVAL; - } - if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled) return -ENODEV; diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index 57541887188b..ee8b4412f7e4 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -407,17 +407,6 @@ static int simplefb_probe(struct platform_device *pdev) struct simplefb_par *par; struct resource *mem; - /* -* Generic drivers must not be registered if a framebuffer exists. -* If a native driver was probed, the display hardware was already -* taken and attempting to use the system framebuffer is dangerous. -*/ - if (num_registered_fb > 0) { - dev_err(&pdev->dev, - "simplefb: a framebuffer is already registered\n"); - return -EINVAL; - } - if (fb_get_options("simplefb", NULL)) return -ENODEV; -- 2.33.0
[PATCH 17/21] fbcon: Move more code into fbcon_release
con2fb_release_oldinfo() has a bunch more kfree() calls than fbcon_exit(), but since kfree() on NULL is harmless doing that in both places should be ok. This is also a bit more symmetric now again with fbcon_open also allocating the fbcon_ops structure. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Tetsuo Handa Cc: Greg Kroah-Hartman Cc: Du Cheng Cc: Claudio Suarez --- drivers/video/fbdev/core/fbcon.c | 33 +--- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index e5e8aaf6f60d..5c14e24d14a1 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -688,6 +688,18 @@ static void fbcon_release(struct fb_info *info) unlock_fb_info(info); module_put(info->fbops->owner); + + if (info->fbcon_par) { + struct fbcon_ops *ops = info->fbcon_par; + + fbcon_del_cursor_work(info); + kfree(ops->cursor_state.mask); + kfree(ops->cursor_data); + kfree(ops->cursor_src); + kfree(ops->fontbuffer); + kfree(info->fbcon_par); + info->fbcon_par = NULL; + } } static int fbcon_open(struct fb_info *info) @@ -741,18 +753,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, static void con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, struct fb_info *newinfo) { - struct fbcon_ops *ops = oldinfo->fbcon_par; int ret; fbcon_release(oldinfo); - fbcon_del_cursor_work(oldinfo); - kfree(ops->cursor_state.mask); - kfree(ops->cursor_data); - kfree(ops->cursor_src); - kfree(ops->fontbuffer); - kfree(oldinfo->fbcon_par); - oldinfo->fbcon_par = NULL; /* If oldinfo and newinfo are driving the same hardware, the fb_release() method of oldinfo may attempt to @@ -3335,19 +3339,8 @@ static void fbcon_exit(void) } } - if (mapped) { - if (info->fbcon_par) { - struct fbcon_ops *ops = info->fbcon_par; - - fbcon_del_cursor_work(info); - kfree(ops->cursor_src); - kfree(ops->cursor_state.mask); - kfree(info->fbcon_par); - info->fbcon_par = NULL; - } - + if (mapped) fbcon_release(info); - } } } -- 2.33.0
[PATCH 12/21] fbcon: Ditch error handling for con2fb_release_oldinfo
It doesn't ever fail anymore. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Thomas Zimmermann Cc: Greg Kroah-Hartman Cc: Claudio Suarez Cc: Du Cheng Cc: Tetsuo Handa --- drivers/video/fbdev/core/fbcon.c | 37 +++- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index eea2ee14b64c..b83a5a77d8a8 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -737,9 +737,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, return err; } -static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, - struct fb_info *newinfo, int unit, - int oldidx, int found) +static void con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, + struct fb_info *newinfo) { struct fbcon_ops *ops = oldinfo->fbcon_par; int ret; @@ -768,8 +767,6 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, "detected unhandled fb_set_par error, " "error code %d\n", ret); } - - return 0; } static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, @@ -823,7 +820,7 @@ static int set_con2fb_map(int unit, int newidx, int user) int oldidx = con2fb_map[unit]; struct fb_info *info = registered_fb[newidx]; struct fb_info *oldinfo = NULL; - int found, err = 0; + int found, err = 0, show_logo; WARN_CONSOLE_UNLOCKED(); @@ -852,18 +849,15 @@ static int set_con2fb_map(int unit, int newidx, int user) * fbcon should release it. */ if (!err && oldinfo && !search_fb_in_map(oldidx)) - err = con2fb_release_oldinfo(vc, oldinfo, info, unit, oldidx, -found); + con2fb_release_oldinfo(vc, oldinfo, info); - if (!err) { - int show_logo = (fg_console == 0 && !user && -logo_shown != FBCON_LOGO_DONTSHOW); + show_logo = (fg_console == 0 && !user && +logo_shown != FBCON_LOGO_DONTSHOW); - if (!found) - fbcon_add_cursor_work(info); - con2fb_map_boot[unit] = newidx; - con2fb_init_display(vc, info, unit, show_logo); - } + if (!found) + fbcon_add_cursor_work(info); + con2fb_map_boot[unit] = newidx; + con2fb_init_display(vc, info, unit, show_logo); if (!search_fb_in_map(info_idx)) info_idx = newidx; @@ -2786,7 +2780,7 @@ static inline void fbcon_unbind(void) {} /* called with console_lock held */ void fbcon_fb_unbind(struct fb_info *info) { - int i, new_idx = -1, ret = 0; + int i, new_idx = -1; int idx = info->node; WARN_CONSOLE_UNLOCKED(); @@ -2820,13 +2814,8 @@ void fbcon_fb_unbind(struct fb_info *info) if (con2fb_map[i] == idx) { con2fb_map[i] = -1; if (!search_fb_in_map(idx)) { - ret = con2fb_release_oldinfo(vc_cons[i].d, -info, NULL, i, -idx, 0); - if (ret) { - con2fb_map[i] = idx; - return; - } + con2fb_release_oldinfo(vc_cons[i].d, + info, NULL); } } } -- 2.33.0
[PATCH 14/21] fbcon: use lock_fb_info in fbcon_open/release
Now we get to the real motiviation, because fbmem.c insists that that's the right lock for these. Ofc fbcon.c has a lot more places where it probably should call lock_fb_info(). But looking at fbmem.c at least most of these seem to be protected by console_lock() too, which is probably what papers over any issues. Note that this means we're shuffling around a bit the locking sections for some of the console takeover and unbind paths, but not all: - console binding/unbinding from the console layer never with lock_fb_info - unbind (as opposed to unlink) never bother with lock_fb_info Also the real serialization against set_par and set_pan are still doing by wrapping the entire ioctl code in console_lock(). So this shuffling shouldn't be worse than what we had from a "can you trigger races?" pov, but it's at least clearer. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Claudio Suarez Cc: Tetsuo Handa Cc: Thomas Zimmermann Cc: Greg Kroah-Hartman Cc: Du Cheng Cc: Sam Ravnborg Cc: Matthew Wilcox Cc: William Kucharski Cc: Alex Deucher Cc: Zheyu Ma Cc: Zhen Lei Cc: Xiyu Yang --- drivers/video/fbdev/core/fbcon.c | 5 + drivers/video/fbdev/core/fbmem.c | 4 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 5a3391ff038d..496bc5f2133e 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -682,8 +682,10 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount) static void fbcon_release(struct fb_info *info) { + lock_fb_info(info); if (info->fbops->fb_release) info->fbops->fb_release(info, 0); + unlock_fb_info(info); module_put(info->fbops->owner); } @@ -695,11 +697,14 @@ static int fbcon_open(struct fb_info *info) if (!try_module_get(info->fbops->owner)) return -ENODEV; + lock_fb_info(info); if (info->fbops->fb_open && info->fbops->fb_open(info, 0)) { + unlock_fb_info(info); module_put(info->fbops->owner); return -ENODEV; } + unlock_fb_info(info); ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); if (!ops) { diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0fa7ede94fa6..fd51d12f2702 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1653,9 +1653,7 @@ static int do_register_framebuffer(struct fb_info *fb_info) console_lock(); else atomic_inc(&ignore_console_lock_warning); - lock_fb_info(fb_info); ret = fbcon_fb_registered(fb_info); - unlock_fb_info(fb_info); if (!lockless_register_fb) console_unlock(); @@ -1672,9 +1670,7 @@ static void unbind_console(struct fb_info *fb_info) return; console_lock(); - lock_fb_info(fb_info); fbcon_fb_unbind(fb_info); - unlock_fb_info(fb_info); console_unlock(); } -- 2.33.0
[PATCH 11/21] fbcon: Extract fbcon_open/release helpers
There's two minor behaviour changes in here: - in error paths we now consistently call fb_ops->fb_release - fb_release really can't fail (fbmem.c ignores it too) and there's no reasonable cleanup we can do anyway. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Claudio Suarez Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Du Cheng --- drivers/video/fbdev/core/fbcon.c | 107 +++ 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index fa30e1909164..eea2ee14b64c 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -680,19 +680,37 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount) #endif /* CONFIG_MISC_TILEBLITTING */ +static int fbcon_open(struct fb_info *info) +{ + if (!try_module_get(info->fbops->owner)) + return -ENODEV; + + if (info->fbops->fb_open && + info->fbops->fb_open(info, 0)) { + module_put(info->fbops->owner); + return -ENODEV; + } + + return 0; +} + +static void fbcon_release(struct fb_info *info) +{ + if (info->fbops->fb_release) + info->fbops->fb_release(info, 0); + + module_put(info->fbops->owner); +} static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, int unit, int oldidx) { struct fbcon_ops *ops = NULL; - int err = 0; - - if (!try_module_get(info->fbops->owner)) - err = -ENODEV; + int err; - if (!err && info->fbops->fb_open && - info->fbops->fb_open(info, 0)) - err = -ENODEV; + err = fbcon_open(info); + if (err) + return err; if (!err) { ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); @@ -713,7 +731,7 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, if (err) { con2fb_map[unit] = oldidx; - module_put(info->fbops->owner); + fbcon_release(info); } return err; @@ -724,45 +742,34 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, int oldidx, int found) { struct fbcon_ops *ops = oldinfo->fbcon_par; - int err = 0, ret; + int ret; - if (oldinfo->fbops->fb_release && - oldinfo->fbops->fb_release(oldinfo, 0)) { - con2fb_map[unit] = oldidx; - if (!found && newinfo->fbops->fb_release) - newinfo->fbops->fb_release(newinfo, 0); - if (!found) - module_put(newinfo->fbops->owner); - err = -ENODEV; - } + fbcon_release(oldinfo); - if (!err) { - fbcon_del_cursor_work(oldinfo); - kfree(ops->cursor_state.mask); - kfree(ops->cursor_data); - kfree(ops->cursor_src); - kfree(ops->fontbuffer); - kfree(oldinfo->fbcon_par); - oldinfo->fbcon_par = NULL; - module_put(oldinfo->fbops->owner); - /* - If oldinfo and newinfo are driving the same hardware, - the fb_release() method of oldinfo may attempt to - restore the hardware state. This will leave the - newinfo in an undefined state. Thus, a call to - fb_set_par() may be needed for the newinfo. - */ - if (newinfo && newinfo->fbops->fb_set_par) { - ret = newinfo->fbops->fb_set_par(newinfo); + fbcon_del_cursor_work(oldinfo); + kfree(ops->cursor_state.mask); + kfree(ops->cursor_data); + kfree(ops->cursor_src); + kfree(ops->fontbuffer); + kfree(oldinfo->fbcon_par); + oldinfo->fbcon_par = NULL; + /* + If oldinfo and newinfo are driving the same hardware, + the fb_release() method of oldinfo may attempt to + restore the hardware state. This will leave the + newinfo in an undefined state. Thus, a call to + fb_set_par() may be needed for the newinfo. + */ + if (newinfo && newinfo->fbops->fb_set_par) { + ret = newinfo->fbops->fb_set_par(newinfo); - if (ret) - printk(KERN_ERR "con2fb_release_oldinfo: " - "detected unhandled fb_set_par error, " - "error code %d\n", ret); - } + if (ret) + printk(KERN_ERR "con2fb_release_oldinfo: " + "detected unhandled fb_set_par error, " + "error code %d\n", ret); } - return err; + return 0; } static void con2fb_init_display(str
[PATCH 16/21] fbcon: Move console_lock for register/unlink/unregister
Ideally console_lock becomes an implementation detail of fbcon.c and doesn't show up anywhere in fbmem.c. We're still pretty far from that, but at least the register/unregister code is there now. With this the do_fb_ioctl() handler is the only code in fbmem.c still calling console_lock(). Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Thomas Zimmermann Cc: Du Cheng Cc: Claudio Suarez Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Matthew Wilcox Cc: Sam Ravnborg Cc: Zheyu Ma Cc: Guenter Roeck Cc: Alex Deucher Cc: Zhen Lei Cc: Xiyu Yang --- drivers/video/fbdev/core/fbcon.c | 33 ++-- drivers/video/fbdev/core/fbmem.c | 23 ++ 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 11b9f962af6f..e5e8aaf6f60d 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2776,10 +2776,12 @@ void fbcon_fb_unbind(struct fb_info *info) int i, new_idx = -1; int idx = info->node; - WARN_CONSOLE_UNLOCKED(); + console_lock(); - if (!fbcon_has_console_bind) + if (!fbcon_has_console_bind) { + console_unlock(); return; + } for (i = first_fb_vc; i <= last_fb_vc; i++) { if (con2fb_map[i] != idx && @@ -2814,6 +2816,8 @@ void fbcon_fb_unbind(struct fb_info *info) } fbcon_unbind(); } + + console_unlock(); } /* called with console_lock held */ @@ -2821,10 +2825,12 @@ void fbcon_fb_unregistered(struct fb_info *info) { int i, idx; - WARN_CONSOLE_UNLOCKED(); + console_lock(); - if (deferred_takeover) + if (deferred_takeover) { + console_unlock(); return; + } idx = info->node; for (i = first_fb_vc; i <= last_fb_vc; i++) { @@ -2853,6 +2859,7 @@ void fbcon_fb_unregistered(struct fb_info *info) if (!num_registered_fb) do_unregister_con_driver(&fb_con); + console_unlock(); } void fbcon_remap_all(struct fb_info *info) @@ -2910,19 +2917,27 @@ static inline void fbcon_select_primary(struct fb_info *info) } #endif /* CONFIG_FRAMEBUFFER_DETECT_PRIMARY */ +static bool lockless_register_fb; +module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400); +MODULE_PARM_DESC(lockless_register_fb, + "Lockless framebuffer registration for debugging [default=off]"); + /* called with console_lock held */ int fbcon_fb_registered(struct fb_info *info) { int ret = 0, i, idx; - WARN_CONSOLE_UNLOCKED(); + if (!lockless_register_fb) + console_lock(); + else + atomic_inc(&ignore_console_lock_warning); idx = info->node; fbcon_select_primary(info); if (deferred_takeover) { pr_info("fbcon: Deferring console take-over\n"); - return 0; + goto out; } if (info_idx == -1) { @@ -2942,6 +2957,12 @@ int fbcon_fb_registered(struct fb_info *info) } } +out: + if (!lockless_register_fb) + console_unlock(); + else + atomic_dec(&ignore_console_lock_warning); + return ret; } diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index fd51d12f2702..904ef1250677 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1573,14 +1573,9 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, } } -static bool lockless_register_fb; -module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400); -MODULE_PARM_DESC(lockless_register_fb, - "Lockless framebuffer registration for debugging [default=off]"); - static int do_register_framebuffer(struct fb_info *fb_info) { - int i, ret; + int i; struct fb_videomode mode; if (fb_check_foreignness(fb_info)) @@ -1649,17 +1644,7 @@ static int do_register_framebuffer(struct fb_info *fb_info) } #endif - if (!lockless_register_fb) - console_lock(); - else - atomic_inc(&ignore_console_lock_warning); - ret = fbcon_fb_registered(fb_info); - - if (!lockless_register_fb) - console_unlock(); - else - atomic_dec(&ignore_console_lock_warning); - return ret; + return fbcon_fb_registered(fb_info); } static void unbind_console(struct fb_info *fb_info) @@ -1669,9 +1654,7 @@ static void unbind_console(struct fb_info *fb_info) if (WARN_ON(i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)) return; - console_lock(); fbcon_fb_unbind(fb_info); - console_unlock(); } static void unlink_framebuffer(struct fb_info *fb_info) @@ -1714,9 +1697,7 @@ static v
[PATCH 15/21] fbcon: Consistently protect deferred_takeover with console_lock()
This shouldn't be a problem in practice since until we've actually taken over the console there's nothing we've registered with the console/vt subsystem, so the exit/unbind path that check this can't do the wrong thing. But it's confusing, so fix it by moving it a tad later. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Du Cheng Cc: Tetsuo Handa Cc: Claudio Suarez Cc: Thomas Zimmermann --- drivers/video/fbdev/core/fbcon.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 496bc5f2133e..11b9f962af6f 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -3247,6 +3247,9 @@ static void fbcon_register_existing_fbs(struct work_struct *work) console_lock(); + deferred_takeover = false; + logo_shown = FBCON_LOGO_DONTSHOW; + for_each_registered_fb(i) fbcon_fb_registered(registered_fb[i]); @@ -3264,8 +3267,6 @@ static int fbcon_output_notifier(struct notifier_block *nb, pr_info("fbcon: Taking over console\n"); dummycon_unregister_output_notifier(&fbcon_output_nb); - deferred_takeover = false; - logo_shown = FBCON_LOGO_DONTSHOW; /* We may get called in atomic context */ schedule_work(&fbcon_deferred_takeover_work); -- 2.33.0
[PATCH 09/21] fbcon: Replace FBCON_FLAGS_INIT with a boolean
It's only one flag and slightly tidier code. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Tetsuo Handa Cc: Greg Kroah-Hartman Cc: Du Cheng Cc: Thomas Zimmermann Cc: Claudio Suarez --- drivers/video/fbdev/core/fbcon.c | 11 +-- drivers/video/fbdev/core/fbcon.h | 4 +--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index affb40658fee..fa30e1909164 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -773,7 +773,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, ops->currcon = fg_console; - if (info->fbops->fb_set_par && !(ops->flags & FBCON_FLAGS_INIT)) { + if (info->fbops->fb_set_par && !ops->initialized) { ret = info->fbops->fb_set_par(info); if (ret) @@ -782,7 +782,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, "error code %d\n", ret); } - ops->flags |= FBCON_FLAGS_INIT; + ops->initialized = true; ops->graphics = 0; fbcon_set_disp(info, &info->var, unit); @@ -1101,8 +1101,7 @@ static void fbcon_init(struct vc_data *vc, int init) * We need to do it in fbcon_init() to prevent screen corruption. */ if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) { - if (info->fbops->fb_set_par && - !(ops->flags & FBCON_FLAGS_INIT)) { + if (info->fbops->fb_set_par && !ops->initialized) { ret = info->fbops->fb_set_par(info); if (ret) @@ -,7 +1110,7 @@ static void fbcon_init(struct vc_data *vc, int init) "error code %d\n", ret); } - ops->flags |= FBCON_FLAGS_INIT; + ops->initialized = true; } ops->graphics = 0; @@ -1186,7 +1185,7 @@ static void fbcon_deinit(struct vc_data *vc) if (con_is_visible(vc)) fbcon_del_cursor_work(info); - ops->flags &= ~FBCON_FLAGS_INIT; + ops->initialized = false; finished: fbcon_free_font(p, free_font); diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h index dce5ce41093e..b18d0cbf73b6 100644 --- a/drivers/video/fbdev/core/fbcon.h +++ b/drivers/video/fbdev/core/fbcon.h @@ -18,8 +18,6 @@ #include -#define FBCON_FLAGS_INIT 1 - /* *This is the interface between the low-level console driver and the *low-level frame buffer device @@ -77,7 +75,7 @@ struct fbcon_ops { intblank_state; intgraphics; intsave_graphics; /* for debug enter/leave */ - intflags; + bool initialized; introtate; intcur_rotate; char *cursor_data; -- 2.33.0
[PATCH 13/21] fbcon: move more common code into fb_open()
No idea why con2fb_acquire_newinfo() initializes much less than fbcon_startup(), but so be it. From a quick look most of the un-initialized stuff should be fairly harmless, but who knows. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Thomas Zimmermann Cc: Claudio Suarez Cc: Du Cheng --- drivers/video/fbdev/core/fbcon.c | 74 +--- 1 file changed, 31 insertions(+), 43 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index b83a5a77d8a8..5a3391ff038d 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -680,8 +680,18 @@ static int fbcon_invalid_charcount(struct fb_info *info, unsigned charcount) #endif /* CONFIG_MISC_TILEBLITTING */ +static void fbcon_release(struct fb_info *info) +{ + if (info->fbops->fb_release) + info->fbops->fb_release(info, 0); + + module_put(info->fbops->owner); +} + static int fbcon_open(struct fb_info *info) { + struct fbcon_ops *ops; + if (!try_module_get(info->fbops->owner)) return -ENODEV; @@ -691,19 +701,22 @@ static int fbcon_open(struct fb_info *info) return -ENODEV; } - return 0; -} + ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); + if (!ops) { + fbcon_release(info); + return -ENOMEM; + } -static void fbcon_release(struct fb_info *info) -{ - if (info->fbops->fb_release) - info->fbops->fb_release(info, 0); + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor); + ops->info = info; + info->fbcon_par = ops; + ops->cur_blink_jiffies = HZ / 5; - module_put(info->fbops->owner); + return 0; } static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, - int unit, int oldidx) + int unit) { struct fbcon_ops *ops = NULL; int err; @@ -712,27 +725,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, if (err) return err; - if (!err) { - ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); - if (!ops) - err = -ENOMEM; - - INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor); - } - - if (!err) { - ops->cur_blink_jiffies = HZ / 5; - ops->info = info; - info->fbcon_par = ops; - - if (vc) - set_blitting_type(vc, info); - } + ops = info->fbcon_par; - if (err) { - con2fb_map[unit] = oldidx; - fbcon_release(info); - } + if (vc) + set_blitting_type(vc, info); return err; } @@ -840,9 +836,11 @@ static int set_con2fb_map(int unit, int newidx, int user) found = search_fb_in_map(newidx); - con2fb_map[unit] = newidx; - if (!err && !found) - err = con2fb_acquire_newinfo(vc, info, unit, oldidx); + if (!err && !found) { + err = con2fb_acquire_newinfo(vc, info, unit); + if (!err) + con2fb_map[unit] = newidx; + } /* * If old fb is not mapped to any of the consoles, @@ -939,20 +937,10 @@ static const char *fbcon_startup(void) if (fbcon_open(info)) return NULL; - ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); - if (!ops) { - fbcon_release(info); - return NULL; - } - - INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor); - + ops = info->fbcon_par; ops->currcon = -1; ops->graphics = 1; ops->cur_rotate = -1; - ops->cur_blink_jiffies = HZ / 5; - ops->info = info; - info->fbcon_par = ops; p->con_rotate = initial_rotation; if (p->con_rotate == -1) @@ -1022,7 +1010,7 @@ static void fbcon_init(struct vc_data *vc, int init) return; if (!info->fbcon_par) - con2fb_acquire_newinfo(vc, info, vc->vc_num, -1); + con2fb_acquire_newinfo(vc, info, vc->vc_num); /* If we are not the first console on this fb, copy the font from that console */ -- 2.33.0
[PATCH 08/21] fbcon: Use delayed work for cursor
Allows us to delete a bunch of hand-rolled stuff. Also to simplify the code we initialize the cursor_work completely when we allocate the fbcon_ops structure, instead of trying to cope with console re-initialization. The motiviation here is that fbcon code stops using the fb_info.queue, which helps with locking issues around cleanup and all that in a later patch. Also note that this allows us to ditch the hand-rolled work cleanup in fbcon_exit - we already call fbcon_del_cursor_timer, which takes care of everything. Plus this was racy anyway. Signed-off-by: Daniel Vetter Cc: Daniel Vetter Cc: Claudio Suarez Cc: Du Cheng Cc: Thomas Zimmermann Cc: Greg Kroah-Hartman Cc: Tetsuo Handa --- drivers/video/fbdev/core/fbcon.c | 85 +--- drivers/video/fbdev/core/fbcon.h | 4 +- 2 files changed, 35 insertions(+), 54 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 814b648e8f09..affb40658fee 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -348,8 +348,8 @@ static int get_color(struct vc_data *vc, struct fb_info *info, static void fb_flashcursor(struct work_struct *work) { - struct fb_info *info = container_of(work, struct fb_info, queue); - struct fbcon_ops *ops = info->fbcon_par; + struct fbcon_ops *ops = container_of(work, struct fbcon_ops, cursor_work.work); + struct fb_info *info; struct vc_data *vc = NULL; int c; int mode; @@ -362,7 +362,10 @@ static void fb_flashcursor(struct work_struct *work) if (ret == 0) return; - if (ops && ops->currcon != -1) + /* protected by console_lock */ + info = ops->info; + + if (ops->currcon != -1) vc = vc_cons[ops->currcon].d; if (!vc || !con_is_visible(vc) || @@ -378,42 +381,25 @@ static void fb_flashcursor(struct work_struct *work) ops->cursor(vc, info, mode, get_color(vc, info, c, 1), get_color(vc, info, c, 0)); console_unlock(); -} -static void cursor_timer_handler(struct timer_list *t) -{ - struct fbcon_ops *ops = from_timer(ops, t, cursor_timer); - struct fb_info *info = ops->info; - - queue_work(system_power_efficient_wq, &info->queue); - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies); + queue_delayed_work(system_power_efficient_wq, &ops->cursor_work, + ops->cur_blink_jiffies); } -static void fbcon_add_cursor_timer(struct fb_info *info) +static void fbcon_add_cursor_work(struct fb_info *info) { struct fbcon_ops *ops = info->fbcon_par; - if ((!info->queue.func || info->queue.func == fb_flashcursor) && - !(ops->flags & FBCON_FLAGS_CURSOR_TIMER) && - !fbcon_cursor_noblink) { - if (!info->queue.func) - INIT_WORK(&info->queue, fb_flashcursor); - - timer_setup(&ops->cursor_timer, cursor_timer_handler, 0); - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies); - ops->flags |= FBCON_FLAGS_CURSOR_TIMER; - } + if (!fbcon_cursor_noblink) + queue_delayed_work(system_power_efficient_wq, &ops->cursor_work, + ops->cur_blink_jiffies); } -static void fbcon_del_cursor_timer(struct fb_info *info) +static void fbcon_del_cursor_work(struct fb_info *info) { struct fbcon_ops *ops = info->fbcon_par; - if (info->queue.func == fb_flashcursor && - ops->flags & FBCON_FLAGS_CURSOR_TIMER) { - del_timer_sync(&ops->cursor_timer); - ops->flags &= ~FBCON_FLAGS_CURSOR_TIMER; - } + cancel_delayed_work_sync(&ops->cursor_work); } #ifndef MODULE @@ -712,6 +698,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info, ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL); if (!ops) err = -ENOMEM; + + INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor); } if (!err) { @@ -749,7 +737,7 @@ static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo, } if (!err) { - fbcon_del_cursor_timer(oldinfo); + fbcon_del_cursor_work(oldinfo); kfree(ops->cursor_state.mask); kfree(ops->cursor_data); kfree(ops->cursor_src); @@ -865,7 +853,7 @@ static int set_con2fb_map(int unit, int newidx, int user) logo_shown != FBCON_LOGO_DONTSHOW); if (!found) - fbcon_add_cursor_timer(info); + fbcon_add_cursor_work(info); con2fb_map_boot[unit] = newidx; con2fb_init_display(vc, info, unit, show_logo); } @@ -962,6 +950,8 @@ static const char *fbcon_startup(void)
[PATCH 07/21] fbdev/sysfs: Fix locking
fb_set_var requires we hold the fb_info lock. Or at least this now matches what the ioctl does ... Note that ps3fb and sh_mobile_lcdcfb are busted in different ways here, but I will not fix them up. Also in practice this isn't a big deal, because really variable fbdev state is actually protected by console_lock (because fbcon just doesn't bother with lock_fb_info() at all), and lock_fb_info protecting anything is really just a neat lie. But that's a much bigger fish to fry. Signed-off-by: Daniel Vetter Cc: Helge Deller Cc: Daniel Vetter Cc: Qing Wang Cc: Sam Ravnborg --- drivers/video/fbdev/core/fbsysfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c index 26892940c213..8c1ee9ecec3d 100644 --- a/drivers/video/fbdev/core/fbsysfs.c +++ b/drivers/video/fbdev/core/fbsysfs.c @@ -91,9 +91,11 @@ static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var) var->activate |= FB_ACTIVATE_FORCE; console_lock(); + lock_fb_info(fb_info); err = fb_set_var(fb_info, var); if (!err) fbcon_update_vcs(fb_info, var->activate & FB_ACTIVATE_ALL); + unlock_fb_info(fb_info); console_unlock(); if (err) return err; -- 2.33.0
[PATCH 10/21] fb: Delete fb_info->queue
It was only used by fbcon, and that now switched to its own, private work. Signed-off-by: Daniel Vetter Cc: Helge Deller Cc: linux-fb...@vger.kernel.org --- include/linux/fb.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/fb.h b/include/linux/fb.h index 02f362c661c8..a8a00d2ba1f3 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -449,7 +449,6 @@ struct fb_info { struct fb_var_screeninfo var; /* Current var */ struct fb_fix_screeninfo fix; /* Current fix */ struct fb_monspecs monspecs;/* Current Monitor specs */ - struct work_struct queue; /* Framebuffer event queue */ struct fb_pixmap pixmap;/* Image hardware mapper */ struct fb_pixmap sprite;/* Cursor hardware mapper */ struct fb_cmap cmap;/* Current cmap */ -- 2.33.0
[PATCH 03/21] fbcon: Restore fbcon scrolling acceleration
This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION option. References: https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de/ Fixes: 39aead8373b3 ("fbcon: Disable accelerated scrolling") Cc: Helge Deller Cc: # v5.11+ Cc: Claudio Suarez Cc: Dave Airlie Cc: Jani Nikula Cc: Linus Torvalds Cc: Linux Fbdev development list Cc: Pavel Machek Cc: Sam Ravnborg Cc: Greg Kroah-Hartman Cc: Javier Martinez Canillas Cc: DRI Development Cc: Linux Kernel Mailing List Cc: Claudio Suarez Cc: Tomi Valkeinen Cc: Geert Uytterhoeven Cc: Thomas Zimmermann Cc: Daniel Vetter Cc: Sven Schnelle Cc: Gerd Hoffmann Signed-off-by: Daniel Vetter --- drivers/video/fbdev/core/fbcon.c | 48 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 2ff90061c7f3..39dc18a5de86 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init) ops->graphics = 0; - /* -* No more hw acceleration for fbcon. -* -* FIXME: Garbage collect all the now dead code after sufficient time -* has passed. -*/ +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION + if ((info->flags & FBINFO_HWACCEL_COPYAREA) && + !(info->flags & FBINFO_HWACCEL_DISABLED)) + p->scrollmode = SCROLL_MOVE; + else /* default to something safe */ + p->scrollmode = SCROLL_REDRAW; +#else p->scrollmode = SCROLL_REDRAW; +#endif /* * ++guenther: console.c:vc_allocate() relies on initializing @@ -1971,15 +1973,49 @@ static void updatescrollmode(struct fbcon_display *p, { struct fbcon_ops *ops = info->fbcon_par; int fh = vc->vc_font.height; + int cap = info->flags; + u16 t = 0; + int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep, + info->fix.xpanstep); + int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t); int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual, info->var.xres_virtual); + int good_pan = (cap & FBINFO_HWACCEL_YPAN) && + divides(ypan, vc->vc_font.height) && vyres > yres; + int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) && + divides(ywrap, vc->vc_font.height) && + divides(vc->vc_font.height, vyres) && + divides(vc->vc_font.height, yres); + int reading_fast = cap & FBINFO_READS_FAST; + int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) && + !(cap & FBINFO_HWACCEL_DISABLED); + int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) && + !(cap & FBINFO_HWACCEL_DISABLED); p->vrows = vyres/fh; if (yres > (fh * (vc->vc_rows + 1))) p->vrows -= (yres - (fh * vc->vc_rows)) / fh; if ((yres % fh) && (vyres % fh < yres % fh)) p->vrows--; + + if (good_wrap || good_pan) { + if (reading_fast || fast_copyarea) + p->scrollmode = good_wrap ? + SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE; + else + p->scrollmode = good_wrap ? SCROLL_REDRAW : + SCROLL_PAN_REDRAW; + } else { + if (reading_fast || (fast_copyarea && !fast_imageblit)) + p->scrollmode = SCROLL_MOVE; + else + p->scrollmode = SCROLL_REDRAW; + } + +#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION + p->scrollmode = SCROLL_REDRAW; +#endif } #define PITCH(w) (((w) + 7) >> 3) -- 2.33.0
[PATCH 06/21] fbcon: delete delayed loading code
Before commit 6104c37094e729f3d4ce65797002112735d49cd1 Author: Daniel Vetter Date: Tue Aug 1 17:32:07 2017 +0200 fbcon: Make fbcon a built-time depency for fbdev it was possible to load fbcon and fbdev drivers in any order, which means that fbcon init had to handle the case where fbdev drivers where already registered. This is no longer possible, hence delete that code. Note that the exit case is a bit more complex and will be done in a separate patch. Signed-off-by: Daniel Vetter Cc: Helge Deller Cc: Daniel Vetter Cc: Claudio Suarez Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Du Cheng --- drivers/video/fbdev/core/fbcon.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 8f971de35885..814b648e8f09 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -942,7 +942,7 @@ static const char *fbcon_startup(void) return display_desc; /* * Instead of blindly using registered_fb[0], we use info_idx, set by -* fb_console_init(); +* fbcon_fb_registered(); */ info = registered_fb[info_idx]; if (!info) @@ -3316,17 +3316,6 @@ static void fbcon_start(void) return; } #endif - - if (num_registered_fb) { - int i; - - for_each_registered_fb(i) { - info_idx = i; - break; - } - - do_fbcon_takeover(0); - } } static void fbcon_exit(void) -- 2.33.0
[PATCH 02/21] fbcon: Resurrect fbcon accelerated scrolling code
This reverts commit b3ec8cdf457e ("fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)"), but with a twist: Because distros like fedora&suse (probably more) really want to move away from fbcon as much as possible, and don't have a need for fancy accelerated fbcon even less, hide the code behind an option. There's also been noises about resurrecting the scrollback code and other pieces, so there's plenty of need for such an option it seems. Compared to direct revert I did move functions around a bit so they're all in one block, to minimize the number of #ifdef. I also tried to stuff this all into a separate file, but that didn't really look much better. I also considered duplicating fbcon_scroll() since that's a bit meh now, but again didn't seem worth the trouble. Maybe when we resurect more code. And finally to shut up unused parameter warnings warnings the macros became static inline. References: https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de/ Fixes: b3ec8cdf457e ("fbdev: Garbage collect fbdev scrolling acceleration, part 1 (from TODO list)") Cc: Claudio Suarez Cc: # v5.16+ Cc: Dave Airlie Cc: Jani Nikula Cc: Linus Torvalds Cc: Linux Fbdev development list Cc: Pavel Machek Cc: Sam Ravnborg Cc: Greg Kroah-Hartman Cc: Javier Martinez Canillas Cc: DRI Development Cc: Linux Kernel Mailing List Cc: Claudio Suarez Cc: Tomi Valkeinen Cc: Geert Uytterhoeven Cc: Thomas Zimmermann Cc: Daniel Vetter Cc: Sven Schnelle Cc: Gerd Hoffmann Signed-off-by: Daniel Vetter --- Documentation/gpu/todo.rst | 13 +- drivers/video/console/Kconfig | 11 + drivers/video/fbdev/core/bitblit.c | 16 + drivers/video/fbdev/core/fbcon.c| 491 +++- drivers/video/fbdev/core/fbcon.h| 59 +++ drivers/video/fbdev/core/fbcon_ccw.c| 28 +- drivers/video/fbdev/core/fbcon_cw.c | 28 +- drivers/video/fbdev/core/fbcon_rotate.h | 21 + drivers/video/fbdev/core/fbcon_ud.c | 37 +- drivers/video/fbdev/core/tileblit.c | 16 + drivers/video/fbdev/skeletonfb.c| 12 +- include/linux/fb.h | 2 +- 12 files changed, 701 insertions(+), 33 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index da138dd39883..29506815d24a 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -303,19 +303,16 @@ Level: Advanced Garbage collect fbdev scrolling acceleration -Scroll acceleration has been disabled in fbcon. Now it works as the old -SCROLL_REDRAW mode. A ton of code was removed in fbcon.c and the hook bmove was -removed from fbcon_ops. -Remaining tasks: +Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode = +SCROLL_REDRAW. There's a ton of code this will allow us to remove: -- a bunch of the hooks in fbcon_ops could be removed or simplified by calling +- lots of code in fbcon.c + +- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called directly instead of the function table (with a switch on p->rotate) - fb_copyarea is unused after this, and can be deleted from all drivers -- after that, fb_copyarea can be deleted from fb_ops in include/linux/fb.h as - well as cfb_copyarea - Note that not all acceleration code can be deleted, since clearing and cursor support is still accelerated, which might be good candidates for further deletion projects. diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index 840d9813b0bc..f83f5c755084 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE help Low-level framebuffer-based console driver. +config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION + bool "Enable legacy fbcon code for hw acceleration" + depends on FRAMEBUFFER_CONSOLE + default n + help +Only enable this options if you are in full control of machine since +the fbcon acceleration code is essentially unmaintained and known +problematic. + +If unsure, select n. + config FRAMEBUFFER_CONSOLE_DETECT_PRIMARY bool "Map the console to the primary display device" depends on FRAMEBUFFER_CONSOLE diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c index 01fae2c96965..f98e8f298bc1 100644 --- a/drivers/video/fbdev/core/bitblit.c +++ b/drivers/video/fbdev/core/bitblit.c @@ -43,6 +43,21 @@ static void update_attr(u8 *dst, u8 *src, int attribute, } } +static void bit_bmove(struct vc_data *vc, struct fb_info *info, int sy, + int sx, int dy, int dx, int height, int width) +{ + struct fb_copyarea area; + + area.sx = sx * vc->vc_font.width; + area.sy = sy * vc->vc_font.height; + area.dx = dx * vc->vc_font.width; + area.dy = dy * vc->vc_font.height; + area.hei
[PATCH 05/21] fbcon: Introduce wrapper for console->fb_info lookup
Half of it is protected by console_lock, but the other half is a lot more awkward: Registration/deregistration of fbdev are serialized, but we don't really clear out anything in con2fb_map and so there's potential for use-after free mixups. First step is to encapsulate the lookup. Signed-off-by: Daniel Vetter Cc: Helge Deller Cc: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Du Cheng Cc: Claudio Suarez Cc: Thomas Zimmermann --- drivers/video/fbdev/core/fbcon.c | 76 ++-- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 2a575620ef59..8f971de35885 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -110,6 +110,18 @@ static struct fbcon_display fb_display[MAX_NR_CONSOLES]; static signed char con2fb_map[MAX_NR_CONSOLES]; static signed char con2fb_map_boot[MAX_NR_CONSOLES]; +static struct fb_info *fbcon_info_from_console(int console) +{ + WARN_CONSOLE_UNLOCKED(); + + /* +* Note that only con2fb_map is protected by the console lock, +* registered_fb is protected by a separate mutex. This lookup can +* therefore race. +*/ + return registered_fb[con2fb_map[console]]; +} + static int logo_lines; /* logo_shown is an index to vc_cons when >= 0; otherwise follows FBCON_LOGO enums. */ @@ -197,7 +209,7 @@ static void fbcon_rotate(struct fb_info *info, u32 rotate) if (!ops || ops->currcon == -1) return; - fb_info = registered_fb[con2fb_map[ops->currcon]]; + fb_info = fbcon_info_from_console(ops->currcon); if (info == fb_info) { struct fbcon_display *p = &fb_display[ops->currcon]; @@ -224,7 +236,7 @@ static void fbcon_rotate_all(struct fb_info *info, u32 rotate) for (i = first_fb_vc; i <= last_fb_vc; i++) { vc = vc_cons[i].d; if (!vc || vc->vc_mode != KD_TEXT || - registered_fb[con2fb_map[i]] != info) + fbcon_info_from_console(i) != info) continue; p = &fb_display[vc->vc_num]; @@ -354,7 +366,7 @@ static void fb_flashcursor(struct work_struct *work) vc = vc_cons[ops->currcon].d; if (!vc || !con_is_visible(vc) || - registered_fb[con2fb_map[vc->vc_num]] != info || + fbcon_info_from_console(vc->vc_num) != info || vc->vc_deccm != 1) { console_unlock(); return; @@ -789,7 +801,7 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info, if (show_logo) { struct vc_data *fg_vc = vc_cons[fg_console].d; struct fb_info *fg_info = - registered_fb[con2fb_map[fg_console]]; + fbcon_info_from_console(fg_console); fbcon_prepare_logo(fg_vc, fg_info, fg_vc->vc_cols, fg_vc->vc_rows, fg_vc->vc_cols, @@ -1012,7 +1024,7 @@ static void fbcon_init(struct vc_data *vc, int init) if (con2fb_map[vc->vc_num] == -1) con2fb_map[vc->vc_num] = info_idx; - info = registered_fb[con2fb_map[vc->vc_num]]; + info = fbcon_info_from_console(vc->vc_num); if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET) logo_shown = FBCON_LOGO_DONTSHOW; @@ -1231,7 +1243,7 @@ static void fbcon_deinit(struct vc_data *vc) static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height, int width) { - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]]; + struct fb_info *info = fbcon_info_from_console(vc->vc_num); struct fbcon_ops *ops = info->fbcon_par; struct fbcon_display *p = &fb_display[vc->vc_num]; @@ -1269,7 +1281,7 @@ static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height, static void fbcon_putcs(struct vc_data *vc, const unsigned short *s, int count, int ypos, int xpos) { - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]]; + struct fb_info *info = fbcon_info_from_console(vc->vc_num); struct fbcon_display *p = &fb_display[vc->vc_num]; struct fbcon_ops *ops = info->fbcon_par; @@ -1289,7 +1301,7 @@ static void fbcon_putc(struct vc_data *vc, int c, int ypos, int xpos) static void fbcon_clear_margins(struct vc_data *vc, int bottom_only) { - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]]; + struct fb_info *info = fbcon_info_from_console(vc->vc_num); struct fbcon_ops *ops = info->fbcon_par; if (!fbcon_is_inactive(vc, info)) @@ -1298,7 +1310,7 @@ static void fbcon_clear_margins(struct vc_data *vc, int bottom_only) static void fbcon_cursor(struct vc_data *vc, int mode) { - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]]; +
[PATCH 04/21] fbcon: delete a few unneeded forward decl
I didn't bother with any code movement to fix the others, these just got a bit in the way. Signed-off-by: Daniel Vetter Cc: Helge Deller Cc: Daniel Vetter Cc: Thomas Zimmermann Cc: Du Cheng Cc: Tetsuo Handa Cc: Claudio Suarez Cc: Greg Kroah-Hartman --- drivers/video/fbdev/core/fbcon.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 39dc18a5de86..2a575620ef59 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -163,18 +163,7 @@ static int fbcon_cursor_noblink; * Interface used by the world */ -static const char *fbcon_startup(void); -static void fbcon_init(struct vc_data *vc, int init); -static void fbcon_deinit(struct vc_data *vc); -static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height, - int width); -static void fbcon_putc(struct vc_data *vc, int c, int ypos, int xpos); -static void fbcon_putcs(struct vc_data *vc, const unsigned short *s, - int count, int ypos, int xpos); static void fbcon_clear_margins(struct vc_data *vc, int bottom_only); -static void fbcon_cursor(struct vc_data *vc, int mode); -static int fbcon_switch(struct vc_data *vc); -static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch); static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table); /* @@ -184,8 +173,8 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var, int unit); static void fbcon_modechanged(struct fb_info *info); static void fbcon_set_all_vcs(struct fb_info *info); -static void fbcon_start(void); static void fbcon_exit(void); + static struct device *fbcon_device; #ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION -- 2.33.0
[PATCH 01/21] MAINTAINERS: Add entry for fbdev core
Ever since Tomi extracted the core code in 2014 it's been defacto me maintaining this, with help from others from dri-devel and sometimes Linus (but those are mostly merge conflicts): $ git shortlog -ns drivers/video/fbdev/core/ | head -n5 35 Daniel Vetter 23 Linus Torvalds 10 Hans de Goede 9 Dave Airlie 6 Peter Rosin I think ideally we'd also record that the various firmware fb drivers (efifb, vesafb, ...) are also maintained in drm-misc because for the past few years the patches have either been to fix handover issues with drm drivers, or caused handover issues with drm drivers. So any other tree just doesn't make sense. But also, there's plenty of outdated MAINTAINER entries for these with people and git trees that haven't been active in years, so maybe let's just leave them alone. And furthermore distros are now adopting simpledrm as the firmware fb driver, so hopefully the need to care about the fbdev firmware drivers will go down going forward. Note that drm-misc is group maintained, I expect that to continue like we've done before, so no new expectations that patches all go through my hands. That would be silly. This also means I'm happy to put any other volunteer's name in the M: line, but otherwise git log says I'm the one who's stuck with this. Cc: Dave Airlie Cc: Jani Nikula Cc: Linus Torvalds Cc: Linux Fbdev development list Cc: Pavel Machek Cc: Sam Ravnborg Cc: Greg Kroah-Hartman Cc: Javier Martinez Canillas Cc: DRI Development Cc: Linux Kernel Mailing List Cc: Claudio Suarez Cc: Tomi Valkeinen Cc: Geert Uytterhoeven Cc: Thomas Zimmermann Cc: Daniel Vetter Cc: Sven Schnelle Cc: Gerd Hoffmann Signed-off-by: Daniel Vetter --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ea3e6c914384..49809eaa3096 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7573,6 +7573,12 @@ S: Maintained W: http://floatingpoint.sourceforge.net/emulator/index.html F: arch/x86/math-emu/ +FRAMEBUFFER CORE +M: Daniel Vetter +F: drivers/video/fbdev/core/ +S: Odd Fixes +T: git git://anongit.freedesktop.org/drm/drm-misc + FRAMEBUFFER LAYER M: Helge Deller L: linux-fb...@vger.kernel.org -- 2.33.0
[PATCH 00/21] some fbcon patches, mostly locking
Hi all, This took way longer than I hoped, but well I got lost in head-scratching locking problems. Anyway ended up typing a pile of fbcon patches. Rough overview: - MAINTAINER entry for fbdev core in drm-misc, with the usual group maintainering - The reverts, but with a compile time option. I looked into combining this with the RFC from Helge, but it looked like worse than either approach, so I've kept mine. I think merging either is fine, no personal stake here on the bikeshed color. - The real distraction, aka a bunch of cleanups and mostly locking work for fbcon. I managed to ditch at least one locking FIXME from fbcon.c. The bad news that I spent a bunch more time pondering about the bigger locking issues in fbcon.c, and I think as-is they're unfixable. We need the threaded printk support to land first, so that we're not adding lock_fb_info() to all printk() contexts. Because that just cannot work. That also means we'll likely have another fbcon regression to face, in the form of worse oops printing ability. And that one I don't think we can fix with clever application of #ifdef, because changing locking rules at compile time in fundamental ways is just not a very good idea. Anyway, testing, review, feedback and all that very much welcome, as usual. Cheers, Daniel Daniel Vetter (21): MAINTAINERS: Add entry for fbdev core fbcon: Resurrect fbcon accelerated scrolling code fbcon: Restore fbcon scrolling acceleration fbcon: delete a few unneeded forward decl fbcon: Introduce wrapper for console->fb_info lookup fbcon: delete delayed loading code fbdev/sysfs: Fix locking fbcon: Use delayed work for cursor fbcon: Replace FBCON_FLAGS_INIT with a boolean fb: Delete fb_info->queue fbcon: Extract fbcon_open/release helpers fbcon: Ditch error handling for con2fb_release_oldinfo fbcon: move more common code into fb_open() fbcon: use lock_fb_info in fbcon_open/release fbcon: Consistently protect deferred_takeover with console_lock() fbcon: Move console_lock for register/unlink/unregister fbcon: Move more code into fbcon_release fbcon: untangle fbcon_exit fbcon: Maintain a private array of fb_info Revert "fbdev: Prevent probing generic drivers if a FB is already registered" fbdev: Make registered_fb[] private to fbmem.c Documentation/gpu/todo.rst | 13 +- MAINTAINERS |6 + drivers/video/console/Kconfig | 11 + drivers/video/fbdev/core/bitblit.c | 16 + drivers/video/fbdev/core/fbcon.c| 1072 +-- drivers/video/fbdev/core/fbcon.h| 67 +- drivers/video/fbdev/core/fbcon_ccw.c| 28 +- drivers/video/fbdev/core/fbcon_cw.c | 28 +- drivers/video/fbdev/core/fbcon_rotate.h | 21 + drivers/video/fbdev/core/fbcon_ud.c | 37 +- drivers/video/fbdev/core/fbmem.c| 35 +- drivers/video/fbdev/core/fbsysfs.c |2 + drivers/video/fbdev/core/tileblit.c | 16 + drivers/video/fbdev/efifb.c | 11 - drivers/video/fbdev/simplefb.c | 11 - drivers/video/fbdev/skeletonfb.c| 12 +- include/linux/fb.h | 10 +- 17 files changed, 1017 insertions(+), 379 deletions(-) -- 2.33.0
[PATCH v4 1/2] drm/msm/dpu: simplify clocks handling
DPU driver contains code to parse clock items from device tree into special data struct and then enable/disable/set rate for the clocks using that data struct. However the DPU driver itself uses only parsing and enabling/disabling part (the rate setting is used by DP driver). Move this implementation to the DP driver (which actually uses rate setting) and replace hand-coded enable/disable/get loops in the DPU with the respective clk_bulk operations. Put operation is removed completely because, it is handled using devres instead. DP implementation is unchanged for now. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/Makefile | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 24 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 46 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 26 +++ .../dpu1/dpu_io_util.c => dp/dp_clk_util.c} | 69 +-- .../dpu1/dpu_io_util.h => dp/dp_clk_util.h} | 8 +-- drivers/gpu/drm/msm/dp/dp_parser.h| 2 +- 9 files changed, 37 insertions(+), 150 deletions(-) rename drivers/gpu/drm/msm/{disp/dpu1/dpu_io_util.c => dp/dp_clk_util.c} (61%) rename drivers/gpu/drm/msm/{disp/dpu1/dpu_io_util.h => dp/dp_clk_util.h} (85%) diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 03ab55c37beb..a44abf0a7660 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -66,7 +66,6 @@ msm-y := \ disp/dpu1/dpu_hw_top.o \ disp/dpu1/dpu_hw_util.o \ disp/dpu1/dpu_hw_vbif.o \ - disp/dpu1/dpu_io_util.o \ disp/dpu1/dpu_kms.o \ disp/dpu1/dpu_mdss.o \ disp/dpu1/dpu_plane.o \ @@ -102,6 +101,7 @@ msm-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_catalog.o \ + dp/dp_clk_util.o \ dp/dp_ctrl.o \ dp/dp_display.o \ dp/dp_drm.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 60fe06018581..4d184122d63e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -284,17 +284,6 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc) } } -static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) -{ - struct dss_clk *core_clk = kms->perf.core_clk; - - if (core_clk->max_rate && (rate > core_clk->max_rate)) - rate = core_clk->max_rate; - - core_clk->rate = rate; - return dev_pm_opp_set_rate(&kms->pdev->dev, core_clk->rate); -} - static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) { u64 clk_rate = kms->perf.perf_tune.min_core_clk; @@ -306,7 +295,7 @@ static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) dpu_cstate = to_dpu_crtc_state(crtc->state); clk_rate = max(dpu_cstate->new_perf.core_clk_rate, clk_rate); - clk_rate = clk_round_rate(kms->perf.core_clk->clk, + clk_rate = clk_round_rate(kms->perf.core_clk, clk_rate); } } @@ -405,10 +394,11 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc, trace_dpu_core_perf_update_clk(kms->dev, stop_req, clk_rate); - ret = _dpu_core_perf_set_core_clk_rate(kms, clk_rate); + if (clk_rate > kms->perf.max_core_clk_rate) + clk_rate = kms->perf.max_core_clk_rate; + ret = dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate); if (ret) { - DPU_ERROR("failed to set %s clock rate %llu\n", - kms->perf.core_clk->clk_name, clk_rate); + DPU_ERROR("failed to set core clock rate %llu\n", clk_rate); return ret; } @@ -529,13 +519,13 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf) int dpu_core_perf_init(struct dpu_core_perf *perf, struct drm_device *dev, struct dpu_mdss_cfg *catalog, - struct dss_clk *core_clk) + struct clk *core_clk) { perf->dev = dev; perf->catalog = catalog; perf->core_clk = core_clk; - perf->max_core_clk_rate = core_clk->max_rate; + perf->max_core_clk_rate = clk_get_rate(core_clk); if (!perf->max_core_clk_rate) { DPU_DEBUG("optional max core clk rate, use default\n"); perf->max_core_clk_rate = DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h index cf4b9b5964c6..8dfcc6db7176 100644 --- a/drivers/g
[PATCH v4 2/2] drm/msm/dp: rewrite dss_module_power to use bulk clock functions
In order to simplify DP code, drop hand-coded loops over clock arrays, replacing them with clk_bulk_* functions. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/dp/dp_clk_util.c | 120 --- drivers/gpu/drm/msm/dp/dp_clk_util.h | 38 - drivers/gpu/drm/msm/dp/dp_ctrl.c | 19 ++--- drivers/gpu/drm/msm/dp/dp_parser.c | 21 - drivers/gpu/drm/msm/dp/dp_parser.h | 17 +++- drivers/gpu/drm/msm/dp/dp_power.c| 91 7 files changed, 100 insertions(+), 207 deletions(-) delete mode 100644 drivers/gpu/drm/msm/dp/dp_clk_util.c delete mode 100644 drivers/gpu/drm/msm/dp/dp_clk_util.h diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index a44abf0a7660..ecf01f9989ed 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -101,7 +101,6 @@ msm-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_catalog.o \ - dp/dp_clk_util.o \ dp/dp_ctrl.o \ dp/dp_display.o \ dp/dp_drm.o \ diff --git a/drivers/gpu/drm/msm/dp/dp_clk_util.c b/drivers/gpu/drm/msm/dp/dp_clk_util.c deleted file mode 100644 index 44a4fc59ff31.. --- a/drivers/gpu/drm/msm/dp/dp_clk_util.c +++ /dev/null @@ -1,120 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2012-2015, 2017-2018, The Linux Foundation. - * All rights reserved. - */ - -#include -#include -#include -#include -#include - -#include - -#include "dp_clk_util.h" - -void msm_dss_put_clk(struct dss_clk *clk_arry, int num_clk) -{ - int i; - - for (i = num_clk - 1; i >= 0; i--) { - if (clk_arry[i].clk) - clk_put(clk_arry[i].clk); - clk_arry[i].clk = NULL; - } -} - -int msm_dss_get_clk(struct device *dev, struct dss_clk *clk_arry, int num_clk) -{ - int i, rc = 0; - - for (i = 0; i < num_clk; i++) { - clk_arry[i].clk = clk_get(dev, clk_arry[i].clk_name); - rc = PTR_ERR_OR_ZERO(clk_arry[i].clk); - if (rc) { - DEV_ERR("%pS->%s: '%s' get failed. rc=%d\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name, rc); - goto error; - } - } - - return rc; - -error: - for (i--; i >= 0; i--) { - if (clk_arry[i].clk) - clk_put(clk_arry[i].clk); - clk_arry[i].clk = NULL; - } - - return rc; -} - -int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk) -{ - int i, rc = 0; - - for (i = 0; i < num_clk; i++) { - if (clk_arry[i].clk) { - if (clk_arry[i].type != DSS_CLK_AHB) { - DEV_DBG("%pS->%s: '%s' rate %ld\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name, - clk_arry[i].rate); - rc = clk_set_rate(clk_arry[i].clk, - clk_arry[i].rate); - if (rc) { - DEV_ERR("%pS->%s: %s failed. rc=%d\n", - __builtin_return_address(0), - __func__, - clk_arry[i].clk_name, rc); - break; - } - } - } else { - DEV_ERR("%pS->%s: '%s' is not available\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name); - rc = -EPERM; - break; - } - } - - return rc; -} - -int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable) -{ - int i, rc = 0; - - if (enable) { - for (i = 0; i < num_clk; i++) { - DEV_DBG("%pS->%s: enable '%s'\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name); - rc = clk_prepare_enable(clk_arry[i].clk); - if (rc) - DEV_ERR("%pS->%s: %s en fail. rc=%d\n", - __builtin_return_address(0), - __func__, - clk_arry[i].clk_name, rc); - - if (rc && i) { - msm_dss_enable_clk(&clk_arry[i - 1], - i - 1, false); - break; -
[PATCH v4 0/2] drm/msm: rework clock handling
msm_dss_clk_*() functions significantly duplicate clk_bulk_* family of functions. Drop custom code and use bulk clocks directly. This also removes dependency of DP driver on the DPU driver internals. Changes since v3: - Switched to devm_clk_bulk_get_all() per Stephen's suggestion. - Removed a call to of_clk_set_defaults() (per Stephen's suggestion again). It duplicates a call in platform_probe(). - Split the first patch (moving helpers to msm_io_utils.c), it's unused now. Changes since v2: - Retain conditional code/prints in DP code to ease debugging - Rebase on top of msm-next and [1] - Split helper functions to msm_io_utils.c as suggested by Jessica Changes since v1: - Rebase on top of current tree to fix conflicts Dmitry Baryshkov (2): drm/msm/dpu: simplify clocks handling drm/msm/dp: rewrite dss_module_power to use bulk clock functions drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 24 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 187 -- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h | 40 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 46 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 26 +-- drivers/gpu/drm/msm/dp/dp_ctrl.c | 19 +- drivers/gpu/drm/msm/dp/dp_parser.c| 21 +- drivers/gpu/drm/msm/dp/dp_parser.h| 17 +- drivers/gpu/drm/msm/dp/dp_power.c | 91 + 12 files changed, 131 insertions(+), 351 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h -- 2.34.1
Re: [PATCH] drm/msm/dp: add wide bus support
On 31/01/2022 23:50, Kuogee Hsieh wrote: On 1/28/2022 8:59 PM, Dmitry Baryshkov wrote: Hi, Thank you for your patch. On Fri, 28 Jan 2022 at 20:29, Kuogee Hsieh wrote: Normally, mdp will push one pixel of data per pixel clock to interface to display. Wide bus feature will increase bus width from 32 bits to 64 bits so that it can push two pixel of data per pixel clock to interface to display. This feature is pre requirement to support 4k resolution. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 2 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 108 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 5 + drivers/gpu/drm/msm/dp/dp_catalog.c | 11 ++- drivers/gpu/drm/msm/dp/dp_catalog.h | 1 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +- drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 17 drivers/gpu/drm/msm/dp/dp_display.h | 3 + drivers/gpu/drm/msm/dp/dp_parser.c | 26 + drivers/gpu/drm/msm/dp/dp_parser.h | 2 + drivers/gpu/drm/msm/msm_drv.h | 9 ++ Can we get this split into logical chunks please? 14 files changed, 190 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1e648db..e2fb5bc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -199,6 +199,8 @@ struct dpu_encoder_virt { struct msm_display_info disp_info; + struct msm_op_info op_info; + bool idle_pc_supported; struct mutex rc_lock; enum dpu_enc_rc_states rc_state; @@ -217,6 +219,13 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc) +{ + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); + + return dpu_enc->op_info.wide_bus_en; +} + static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned bpc) { struct dpu_hw_dither_cfg dither_cfg = { 0 }; @@ -2112,6 +2121,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + struct msm_op_info *op_info; int ret = 0; dpu_enc = to_dpu_encoder_virt(enc); @@ -2128,8 +2138,12 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, timer_setup(&dpu_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) + else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) { dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]]; + op_info = &priv->op_info[disp_info->h_tile_instance[0]]; op_info should be defined per INTF rather than per h_tile. This way you won't have to check for intf_type here. + dpu_enc->op_info = *op_info; So... we set this data in msm_drm_private only to copy it to dpu_encoder_virt? Please don't do this. Allow one to query the data from the DP rather than blindly copying it over and over again. + + } INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index e241914..0d73550 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc); */ int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc); +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index ddd9d89..04ac2dc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params( timing->v_back_porch += timing->v_front_porch; timing->v_front_porch = 0; } + + timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent); + + /* + * for DP, divide the horizonal parameters by 2 when + * widebus is enabled + */ + if (phys_enc->hw_intf->cap->type == INTF_DP &&
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Hi Javier, On Mon, Jan 31, 2022 at 09:12:20PM +0100, Javier Martinez Canillas wrote: > This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, > SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. > > Using the DRM fb emulation, all the tests from Geert Uytterhoeven's fbtest > (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes: Impressed how fast you did this! Saw the picture you posted a link to on irc - nice. > Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x > (which would be more accurate) to avoid confusion for users who want to > migrate from the existing ssd1307fb fbdev driver. Looking forward the name ssd130x would make more sense. There is only so many existing users and a potential of much more new users. So in my color of the world the naming that benefits the most users wins. Sam
Re: [PATCH 1/4] drm: Add I2C connector type
On Mon, Jan 31, 2022 at 09:12:21PM +0100, Javier Martinez Canillas wrote: > There isn't a connector type for display controllers accesed through I2C, > most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. > > Add an I2C connector type to match the actual connector. > > As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector > type"), user-space should be able to cope with a connector type that does > not yet understand. > > Tested with `modetest -M ssd1307 -c` and shows the connector as unknown-1. I had expected unknown-21?? > > Signed-off-by: Javier Martinez Canillas Reviewed-by: Sam Ravnborg > --- > > drivers/gpu/drm/drm_connector.c | 1 + > include/uapi/drm/drm_mode.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index a50c82bc2b2f..975a7525a508 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -105,6 +105,7 @@ static struct drm_conn_prop_enum_list > drm_connector_enum_list[] = { > { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" }, > { DRM_MODE_CONNECTOR_SPI, "SPI" }, > { DRM_MODE_CONNECTOR_USB, "USB" }, > + { DRM_MODE_CONNECTOR_I2C, "I2C" }, > }; > > void drm_connector_ida_init(void) > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index e1e351682872..d6d6288242db 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -421,6 +421,7 @@ enum drm_mode_subconnector { > #define DRM_MODE_CONNECTOR_WRITEBACK 18 > #define DRM_MODE_CONNECTOR_SPI 19 > #define DRM_MODE_CONNECTOR_USB 20 > +#define DRM_MODE_CONNECTOR_I2C 21 > > /** > * struct drm_mode_get_connector - Get connector metadata. > -- > 2.34.1
Re: [PATCH] drm/msm/dp: add wide bus support
On 1/28/2022 8:59 PM, Dmitry Baryshkov wrote: Hi, Thank you for your patch. On Fri, 28 Jan 2022 at 20:29, Kuogee Hsieh wrote: Normally, mdp will push one pixel of data per pixel clock to interface to display. Wide bus feature will increase bus width from 32 bits to 64 bits so that it can push two pixel of data per pixel clock to interface to display. This feature is pre requirement to support 4k resolution. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 16 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 2 + .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 14 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 108 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 5 + drivers/gpu/drm/msm/dp/dp_catalog.c| 11 ++- drivers/gpu/drm/msm/dp/dp_catalog.h| 1 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +- drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c| 17 drivers/gpu/drm/msm/dp/dp_display.h| 3 + drivers/gpu/drm/msm/dp/dp_parser.c | 26 + drivers/gpu/drm/msm/dp/dp_parser.h | 2 + drivers/gpu/drm/msm/msm_drv.h | 9 ++ Can we get this split into logical chunks please? 14 files changed, 190 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 1e648db..e2fb5bc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -199,6 +199,8 @@ struct dpu_encoder_virt { struct msm_display_info disp_info; + struct msm_op_info op_info; + bool idle_pc_supported; struct mutex rc_lock; enum dpu_enc_rc_states rc_state; @@ -217,6 +219,13 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = { 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10 }; +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc) +{ + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc); + + return dpu_enc->op_info.wide_bus_en; +} + static void _dpu_encoder_setup_dither(struct dpu_hw_pingpong *hw_pp, unsigned bpc) { struct dpu_hw_dither_cfg dither_cfg = { 0 }; @@ -2112,6 +2121,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + struct msm_op_info *op_info; int ret = 0; dpu_enc = to_dpu_encoder_virt(enc); @@ -2128,8 +2138,12 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, timer_setup(&dpu_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) + else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS) { dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]]; + op_info = &priv->op_info[disp_info->h_tile_instance[0]]; op_info should be defined per INTF rather than per h_tile. This way you won't have to check for intf_type here. + dpu_enc->op_info = *op_info; So... we set this data in msm_drm_private only to copy it to dpu_encoder_virt? Please don't do this. Allow one to query the data from the DP rather than blindly copying it over and over again. + + } INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index e241914..0d73550 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -168,4 +168,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc); */ int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc); +bool dpu_encoder_is_widebus_enabled(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index ddd9d89..04ac2dc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -110,6 +110,20 @@ static void drm_mode_to_intf_timing_params( timing->v_back_porch += timing->v_front_porch; timing->v_front_porch = 0; } + + timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent); + + /* +* for DP, divide the horizonal parameters by 2 when +* widebus is enabled +*/ + if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) { What about INTF_eDP? I suspect
Re: [PATCH 4/4] MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver
Hi Javier, On Mon, Jan 31, 2022 at 09:15:37PM +0100, Javier Martinez Canillas wrote: > To make sure that tools like the get_maintainer.pl script will suggest > to Cc me if patches are posted for this driver. > > Also include the Device Tree binding for the old ssd1307fb fbdev driver > since the new DRM driver was made compatible with the existing binding. To avoid any confusion add yourself as Maintainer in the solomon,ssd1307fb.yaml file too. With that done: Acked-by: Sam Ravnborg > > Signed-off-by: Javier Martinez Canillas > --- > > MAINTAINERS | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index d03ad8da1f36..2e6c3aad5d71 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6102,6 +6102,13 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/display/repaper.txt > F: drivers/gpu/drm/tiny/repaper.c > > +DRM DRIVER FOR SOLOMON SSD1307 OLED DISPLAYS > +M: Javier Martinez Canillas > +S: Maintained > +T: git git://anongit.freedesktop.org/drm/drm-misc > +F: Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > +F: drivers/gpu/drm/tiny/ssd1307.c > + > DRM DRIVER FOR QEMU'S CIRRUS DEVICE > M: Dave Airlie > M: Gerd Hoffmann > -- > 2.34.1
Re: [PATCH] dma-resv: some doc polish for iterators
On Tue, Nov 30, 2021 at 04:27:55PM +0100, Daniel Vetter wrote: > Hammer it a bit more in that iterators can be restarted and when that > matters, plus suggest to prefer the locked version whenver. > > Also delete the two leftover kerneldoc for static functions plus > sprinkle some more links while at it. > > v2: Keep some comments (Christian) > > Reviewed-by: Christian König > Signed-off-by: Daniel Vetter > Cc: Sumit Semwal > Cc: "Christian König" > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org Soo behind on random stuff, just noticed I never merged this. Done that now. -Daniel > --- > drivers/dma-buf/dma-resv.c | 29 +++-- > include/linux/dma-resv.h | 13 - > 2 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 9eb2baa387d4..a62eb8fc33b9 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -323,12 +323,8 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, > struct dma_fence *fence) > } > EXPORT_SYMBOL(dma_resv_add_excl_fence); > > -/** > - * dma_resv_iter_restart_unlocked - restart the unlocked iterator > - * @cursor: The dma_resv_iter object to restart > - * > - * Restart the unlocked iteration by initializing the cursor object. > - */ > +/* Restart the iterator by initializing all the necessary fields, but not the > + * relation to the dma_resv object. */ > static void dma_resv_iter_restart_unlocked(struct dma_resv_iter *cursor) > { > cursor->seq = read_seqcount_begin(&cursor->obj->seq); > @@ -344,14 +340,7 @@ static void dma_resv_iter_restart_unlocked(struct > dma_resv_iter *cursor) > cursor->is_restarted = true; > } > > -/** > - * dma_resv_iter_walk_unlocked - walk over fences in a dma_resv obj > - * @cursor: cursor to record the current position > - * > - * Return all the fences in the dma_resv object which are not yet signaled. > - * The returned fence has an extra local reference so will stay alive. > - * If a concurrent modify is detected the whole iteration is started over > again. > - */ > +/* Walk to the next not signaled fence and grab a reference to it */ > static void dma_resv_iter_walk_unlocked(struct dma_resv_iter *cursor) > { > struct dma_resv *obj = cursor->obj; > @@ -387,6 +376,12 @@ static void dma_resv_iter_walk_unlocked(struct > dma_resv_iter *cursor) > * dma_resv_iter_first_unlocked - first fence in an unlocked dma_resv obj. > * @cursor: the cursor with the current position > * > + * Subsequent fences are iterated with dma_resv_iter_next_unlocked(). > + * > + * Beware that the iterator can be restarted. Code which accumulates > statistics > + * or similar needs to check for this with dma_resv_iter_is_restarted(). For > + * this reason prefer the locked dma_resv_iter_first() whenver possible. > + * > * Returns the first fence from an unlocked dma_resv obj. > */ > struct dma_fence *dma_resv_iter_first_unlocked(struct dma_resv_iter *cursor) > @@ -406,6 +401,10 @@ EXPORT_SYMBOL(dma_resv_iter_first_unlocked); > * dma_resv_iter_next_unlocked - next fence in an unlocked dma_resv obj. > * @cursor: the cursor with the current position > * > + * Beware that the iterator can be restarted. Code which accumulates > statistics > + * or similar needs to check for this with dma_resv_iter_is_restarted(). For > + * this reason prefer the locked dma_resv_iter_next() whenver possible. > + * > * Returns the next fence from an unlocked dma_resv obj. > */ > struct dma_fence *dma_resv_iter_next_unlocked(struct dma_resv_iter *cursor) > @@ -431,6 +430,8 @@ EXPORT_SYMBOL(dma_resv_iter_next_unlocked); > * dma_resv_iter_first - first fence from a locked dma_resv object > * @cursor: cursor to record the current position > * > + * Subsequent fences are iterated with dma_resv_iter_next_unlocked(). > + * > * Return the first fence in the dma_resv object while holding the > * &dma_resv.lock. > */ > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h > index dbd235ab447f..ebe908592ac3 100644 > --- a/include/linux/dma-resv.h > +++ b/include/linux/dma-resv.h > @@ -153,6 +153,13 @@ struct dma_resv { > * struct dma_resv_iter - current position into the dma_resv fences > * > * Don't touch this directly in the driver, use the accessor function > instead. > + * > + * IMPORTANT > + * > + * When using the lockless iterators like dma_resv_iter_next_unlocked() or > + * dma_resv_for_each_fence_unlocked() beware that the iterator can be > restarted. > + * Code which accumulates statistics or similar needs to check for this with > + * dma_resv_iter_is_restarted(). > */ > struct dma_resv_iter { > /** @obj: The dma_resv object we iterate over */ > @@ -243,7 +250,11 @@ static inline bool dma_resv_iter_is_restarted(struct > dma_resv_iter *cursor) > * &dma_resv.lock and using RCU instead. The cursor needs to be initialized > * with dma_resv_iter_beg
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
Hi all, On 2021-12-22 19:31, Dmitry Osipenko wrote: 22.12.2021 22:30, Jon Hunter пишет: On 22/12/2021 19:01, Dmitry Osipenko wrote: ... diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index e08e331e46ae..8194826c9ce3 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -137,6 +137,15 @@ void host1x_syncpt_restore(struct host1x *host) struct host1x_syncpt *sp_base = host->syncpt; unsigned int i; + for (i = 0; i < host->info->nb_pts; i++) { + /* + * Unassign syncpt from channels for purposes of Tegra186 + * syncpoint protection. This prevents any channel from + * accessing it until it is reassigned. + */ + host1x_hw_syncpt_assign_to_channel(host, sp_base + i, NULL); + } + for (i = 0; i < host1x_syncpt_nb_pts(host); i++) host1x_hw_syncpt_restore(host, sp_base + i); @@ -352,13 +361,6 @@ int host1x_syncpt_init(struct host1x *host) for (i = 0; i < host->info->nb_pts; i++) { syncpt[i].id = i; syncpt[i].host = host; - - /* - * Unassign syncpt from channels for purposes of Tegra186 - * syncpoint protection. This prevents any channel from - * accessing it until it is reassigned. - */ - host1x_hw_syncpt_assign_to_channel(host, &syncpt[i], NULL); } for (i = 0; i < host->info->nb_bases; i++) Thanks! This fixed it! I'll prepare proper patch with yours t-b, thank you. The fix has been in -next for some time now, but it still hasn't made it into Linus' tree (at least not in -rc2). Any hope for this to land -rc3? Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH] dma-buf: heaps: Fix potential spectre v1 gadget
On Sat, Jan 29, 2022 at 7:06 AM Jordy Zomer wrote: > > It appears like nr could be a Spectre v1 gadget as it's supplied by a > user and used as an array index. Prevent the contents > of kernel memory from being leaked to userspace via speculative > execution by using array_index_nospec. > > Signed-off-by: Jordy Zomer > --- > drivers/dma-buf/dma-heap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > index 56bf5ad01ad5..8f5848aa144f 100644 > --- a/drivers/dma-buf/dma-heap.c > +++ b/drivers/dma-buf/dma-heap.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -135,6 +136,7 @@ static long dma_heap_ioctl(struct file *file, unsigned > int ucmd, > if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds)) > return -EINVAL; > > + nr = array_index_nospec(nr, ARRAY_SIZE(dma_heap_ioctl_cmds)); > /* Get the kernel ioctl cmd that matches */ > kcmd = dma_heap_ioctl_cmds[nr]; Thanks for submitting this! It looks sane to me. Acked-by: John Stultz thanks -john
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
On Monday, January 31st, 2022 at 21:36, Simon Ser wrote: > This driver only advertises XRGB in ssd1307_formats. It would be nice to > expose R8 as well so that user-space can directly produce suitable buffers. > It would also be nice to have some kind of preferred format, so that > user-space > knows R8 is preferred over XRGB. Hm, since the format used by the hw is actually R1, adding that to drm_fourcc.h would be even better. Let me know if you want me to type up any of the user-space bits.
Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
This driver only advertises XRGB in ssd1307_formats. It would be nice to expose R8 as well so that user-space can directly produce suitable buffers. It would also be nice to have some kind of preferred format, so that user-space knows R8 is preferred over XRGB.
[PATCH 3/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
Add a DRM driver for SSD1305, SSD1306, SSD1307 and SSD1309 Solomon OLED controllers that can be programmed via an I2C interface. This is a port of the ssd1307fb driver that already supports these devices. A Device Tree binding is not added because the DRM driver is compatible with the existing binding for the ssd1307fb driver. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/tiny/Kconfig | 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/ssd1307.c | 976 + 3 files changed, 989 insertions(+) create mode 100644 drivers/gpu/drm/tiny/ssd1307.c diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 712e0004e96e..25c9c013bcda 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -157,6 +157,18 @@ config TINYDRM_REPAPER If M is selected the module will be called repaper. +config TINYDRM_SSD1307 + tristate "DRM support for Solomon SSD1307 OLED displays" + depends on DRM && I2C + select DRM_KMS_HELPER + select DRM_GEM_SHMEM_HELPER + select BACKLIGHT_CLASS_DEVICE + help + DRM driver for the SSD1305, SSD1306, SSD1307 and SSD1309 Solomon + OLED controllers that can be programmed via an I2C interface. + + If M is selected the module will be called ssd1307. + config TINYDRM_ST7586 tristate "DRM support for Sitronix ST7586 display panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 5d5505d40e7b..38c4ce96 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -12,5 +12,6 @@ obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o +obj-$(CONFIG_TINYDRM_SSD1307) += ssd1307.o obj-$(CONFIG_TINYDRM_ST7586) += st7586.o obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o diff --git a/drivers/gpu/drm/tiny/ssd1307.c b/drivers/gpu/drm/tiny/ssd1307.c new file mode 100644 index ..4ea223674587 --- /dev/null +++ b/drivers/gpu/drm/tiny/ssd1307.c @@ -0,0 +1,976 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * DRM driver for Solomon SSD1307 OLED displays + * + * Copyright 2022 Red Hat Inc. + * + * Based on drivers/video/fbdev/ssd1307fb.c + * Copyright 2012 Free Electrons + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_NAME"ssd1307" +#define DRIVER_DESC"DRM driver for Solomon SSD1307 OLED displays" +#define DRIVER_DATE"20220131" +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 + +#define SSD1307_DATA 0x40 +#define SSD1307_COMMAND0x80 + +#define SSD1307_SET_ADDRESS_MODE 0x20 +#define SSD1307_SET_ADDRESS_MODE_HORIZONTAL(0x00) +#define SSD1307_SET_ADDRESS_MODE_VERTICAL (0x01) +#define SSD1307_SET_ADDRESS_MODE_PAGE (0x02) +#define SSD1307_SET_COL_RANGE 0x21 +#define SSD1307_SET_PAGE_RANGE 0x22 +#define SSD1307_CONTRAST 0x81 +#define SSD1307_SET_LOOKUP_TABLE 0x91 +#define SSD1307_CHARGE_PUMP0x8d +#define SSD1307_SEG_REMAP_ON 0xa1 +#define SSD1307_DISPLAY_OFF0xae +#define SSD1307_SET_MULTIPLEX_RATIO0xa8 +#define SSD1307_DISPLAY_ON 0xaf +#define SSD1307_START_PAGE_ADDRESS 0xb0 +#define SSD1307_SET_DISPLAY_OFFSET 0xd3 +#define SSD1307_SET_CLOCK_FREQ 0xd5 +#define SSD1307_SET_AREA_COLOR_MODE0xd8 +#define SSD1307_SET_PRECHARGE_PERIOD 0xd9 +#define SSD1307_SET_COM_PINS_CONFIG0xda +#define SSD1307_SET_VCOMH 0xdb + +#define MAX_CONTRAST 255 + +struct ssd1307_deviceinfo { + u32 default_vcomh; + u32 default_dclk_div; + u32 default_dclk_frq; + int need_pwm; + int need_chargepump; +}; + +struct ssd1307_device { + struct drm_device drm; + struct drm_simple_display_pipe pipe; + struct drm_display_mode mode; + struct drm_connector connector; + struct i2c_client *client; + + const struct ssd1307_deviceinfo *device_info; + + unsigned area_color_enable : 1; + unsigned com_invdir : 1; + unsigned com_lrremap : 1; + unsigned com_seq : 1; + unsigned lookup_table_set : 1; + unsigned low_power : 1; + unsigned seg_remap : 1; + u32 com_offset; + u32 contrast; + u32 dclk_div; + u32 dclk_frq; + u32 height; + u
[PATCH 4/4] MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver
To make sure that tools like the get_maintainer.pl script will suggest to Cc me if patches are posted for this driver. Also include the Device Tree binding for the old ssd1307fb fbdev driver since the new DRM driver was made compatible with the existing binding. Signed-off-by: Javier Martinez Canillas --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index d03ad8da1f36..2e6c3aad5d71 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6102,6 +6102,13 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/repaper.txt F: drivers/gpu/drm/tiny/repaper.c +DRM DRIVER FOR SOLOMON SSD1307 OLED DISPLAYS +M: Javier Martinez Canillas +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +F: drivers/gpu/drm/tiny/ssd1307.c + DRM DRIVER FOR QEMU'S CIRRUS DEVICE M: Dave Airlie M: Gerd Hoffmann -- 2.34.1
Re: [PATCH -next] drm/amd/display: clean up some inconsistent indenting
Applied. Thanks! Alex On Mon, Jan 31, 2022 at 10:17 AM Harry Wentland wrote: > > On 2022-01-28 20:04, Yang Li wrote: > > Eliminate the follow smatch warning: > > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c:2246 > > dp_perform_8b_10b_link_training() warn: inconsistent indenting > > > > Reported-by: Abaci Robot > > Signed-off-by: Yang Li > > Reviewed-by: Harry Wentland > > Harry > > > --- > > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > index daaec3164875..34ffcd5bb1d7 100644 > > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > @@ -2243,11 +2243,11 @@ static enum link_training_result > > dp_perform_8b_10b_link_training( > > > > if (status == LINK_TRAINING_SUCCESS) { > > status = perform_clock_recovery_sequence(link, link_res, > > lt_settings, DPRX); > > - if (status == LINK_TRAINING_SUCCESS) { > > - status = perform_channel_equalization_sequence(link, > > - link_res, > > - lt_settings, > > - DPRX); > > + if (status == LINK_TRAINING_SUCCESS) { > > + status = perform_channel_equalization_sequence(link, > > + > > link_res, > > + > > lt_settings, > > +DPRX); > > } > > } > > >
[PATCH 1/4] drm: Add I2C connector type
There isn't a connector type for display controllers accesed through I2C, most drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. Add an I2C connector type to match the actual connector. As Noralf Trønnes mentions in commit fc06bf1d76d6 ("drm: Add SPI connector type"), user-space should be able to cope with a connector type that does not yet understand. Tested with `modetest -M ssd1307 -c` and shows the connector as unknown-1. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/drm_connector.c | 1 + include/uapi/drm/drm_mode.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index a50c82bc2b2f..975a7525a508 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -105,6 +105,7 @@ static struct drm_conn_prop_enum_list drm_connector_enum_list[] = { { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" }, { DRM_MODE_CONNECTOR_SPI, "SPI" }, { DRM_MODE_CONNECTOR_USB, "USB" }, + { DRM_MODE_CONNECTOR_I2C, "I2C" }, }; void drm_connector_ida_init(void) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index e1e351682872..d6d6288242db 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -421,6 +421,7 @@ enum drm_mode_subconnector { #define DRM_MODE_CONNECTOR_WRITEBACK 18 #define DRM_MODE_CONNECTOR_SPI 19 #define DRM_MODE_CONNECTOR_USB 20 +#define DRM_MODE_CONNECTOR_I2C 21 /** * struct drm_mode_get_connector - Get connector metadata. -- 2.34.1
[PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays
This patch series adds a DRM driver for the Solomon OLED SSD1305, SSD1306, SSD1307 and SSD1309 displays. It is a port of the ssd1307fb fbdev driver. Using the DRM fb emulation, all the tests from Geert Uytterhoeven's fbtest (https://git.kernel.org/pub/scm/linux/kernel/git/geert/fbtest.git) passes: ./fbtest -f /dev/fb1 Using drawops cfb32 (32 bpp packed pixels) Available visuals: Monochrome Grayscale 256 Truecolor 8:8:8:0 Using visops truecolor Running all tests test001: PASSED test002: PASSED test003: PASSED test004: PASSED test005: PASSED test006: PASSED test008: PASSED test009: PASSED test010: PASSED Benchmarking... 10x10 squares: 412.99 Mpixels/s Benchmarking... 20x20 squares: 857.46 Mpixels/s Benchmarking... 50x50 squares: 1593.51 Mpixels/s test012: PASSED Benchmarking... R5 circles: 237.07 Mpixels/s Benchmarking... R10 circles: 501.24 Mpixels/s Benchmarking... R25 circles: 947.86 Mpixels/s test013: PASSED Patch #1 adds an I2C connector type since currently there isn't one and I2C drivers use DRM_MODE_CONNECTOR_Unknown or DRM_MODE_CONNECTOR_VIRTUAL. Patch #2 adds a drm_fb_gray8_to_mono_reversed() DRM format helper since most DRM/KMS user-space don't support bpp 1 displays, so drivers expose a common format that's converted to greyscale and then to monochrome. Patch #3 adds the driver. The name ssd1307 was used instead of ssd130x (which would be more accurate) to avoid confusion for users who want to migrate from the existing ssd1307fb fbdev driver. Patch #4 just adds a MAINTAINERS entry for this new DRM driver. Best regards, Javier Javier Martinez Canillas (4): drm: Add I2C connector type drm/format-helper: Add drm_fb_gray8_to_mono_reversed() drm/tiny: Add driver for Solomon SSD1307 OLED displays MAINTAINERS: Add entry for Solomon SSD1307 OLED displays DRM driver MAINTAINERS | 7 + drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_format_helper.c | 35 + drivers/gpu/drm/tiny/Kconfig| 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/ssd1307.c | 976 include/drm/drm_format_helper.h | 2 + include/uapi/drm/drm_mode.h | 1 + 8 files changed, 1035 insertions(+) create mode 100644 drivers/gpu/drm/tiny/ssd1307.c -- 2.34.1
[PATCH 2/4] drm/format-helper: Add drm_fb_gray8_to_mono_reversed()
Add support to convert 8-bit grayscale to reversed monochrome for drivers that control monochromatic displays, that only have 1 bit per pixel depth. This helper function was based on repaper_gray8_to_mono_reversed() from the drivers/gpu/drm/tiny/repaper.c driver. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/drm_format_helper.c | 35 + include/drm/drm_format_helper.h | 2 ++ 2 files changed, 37 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index 0f28dd2bdd72..bf477c136082 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -584,3 +584,38 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for return -EINVAL; } EXPORT_SYMBOL(drm_fb_blit_toio); + +/** + * drm_fb_gray8_to_mono_reversed - Convert grayscale to reversed monochrome + * @dst: reversed monochrome destination buffer + * @src: 8-bit grayscale source buffer + * @clip: Clip rectangle area to copy + * + * DRM doesn't have native monochrome or grayscale support. + * Such drivers can announce the commonly supported XR24 format to userspace + * and use drm_fb_xrgb_to_gray8() to convert to grayscale and then this + * helper function to convert to the native format. + */ +void drm_fb_gray8_to_mono_reversed(void *dst, void *src, const struct drm_rect *clip) +{ + size_t width = drm_rect_width(clip); + size_t height = drm_rect_width(clip); + + u8 *mono = dst, *gray8 = src; + unsigned int y, xb, i; + + for (y = 0; y < height; y++) + for (xb = 0; xb < width / 8; xb++) { + u8 byte = 0x00; + + for (i = 0; i < 8; i++) { + int x = xb * 8 + i; + + byte >>= 1; + if (gray8[y * width + x] >> 7) + byte |= BIT(7); + } + *mono++ = byte; + } +} +EXPORT_SYMBOL(drm_fb_gray8_to_mono_reversed); diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index b30ed5de0a33..cd4c8b7c78de 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -43,4 +43,6 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for const void *vmap, const struct drm_framebuffer *fb, const struct drm_rect *rect); +void drm_fb_gray8_to_mono_reversed(void *dst, void *vaddr, const struct drm_rect *clip); + #endif /* __LINUX_DRM_FORMAT_HELPER_H */ -- 2.34.1
[PATCH] mm: add device coherent vma selection for memory migration
This case is used to migrate pages from device memory, back to system memory. Device coherent type memory is cache coherent from device and CPU point of view. Signed-off-by: Alex Sierra Acked-by: Felix Kuehling --- v2: condition added when migrations from device coherent pages. --- include/linux/migrate.h | 1 + mm/migrate.c| 12 +--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/migrate.h b/include/linux/migrate.h index db96e10eb8da..66a34eae8cb6 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -130,6 +130,7 @@ static inline unsigned long migrate_pfn(unsigned long pfn) enum migrate_vma_direction { MIGRATE_VMA_SELECT_SYSTEM = 1 << 0, MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1, + MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2, }; struct migrate_vma { diff --git a/mm/migrate.c b/mm/migrate.c index cd137aedcfe5..69c6830c47c6 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2264,15 +2264,21 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, if (is_writable_device_private_entry(entry)) mpfn |= MIGRATE_PFN_WRITE; } else { - if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) - goto next; pfn = pte_pfn(pte); - if (is_zero_pfn(pfn)) { + if (is_zero_pfn(pfn) && + (migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) { mpfn = MIGRATE_PFN_MIGRATE; migrate->cpages++; goto next; } page = vm_normal_page(migrate->vma, addr, pte); + if (page && !is_zone_device_page(page) && + !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) + goto next; + else if (page && is_device_coherent_page(page) && + (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_COHERENT) || +page->pgmap->owner != migrate->pgmap_owner)) + goto next; mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE; mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0; } -- 2.32.0
[PATCH v6 1/5] drm/i915: add needs_compact_pt flag
From: Ramalingam C Add a new platform flag, needs_compact_pt, to mark the requirement of compact pt layout support for the ppGTT when using 64K GTT pages. With this flag has_64k_pages will only indicate requirement of 64K GTT page sizes or larger for device local memory access. v6: * minor doc formatting Suggested-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/i915_drv.h | 11 --- drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 00e7594b59c9..4afdfa5fd3b3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1512,12 +1512,17 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, /* * Set this flag, when platform requires 64K GTT page sizes or larger for - * device local memory access. Also this flag implies that we require or - * at least support the compact PT layout for the ppGTT when using the 64K - * GTT pages. + * device local memory access. */ #define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages) +/* + * Set this flag when platform doesn't allow both 64k pages and 4k pages in + * the same PT. this flag means we need to support compact PT layout for the + * ppGTT when using the 64K GTT pages. + */ +#define NEEDS_COMPACT_PT(dev_priv) (INTEL_INFO(dev_priv)->needs_compact_pt) + #define HAS_IPC(dev_priv) (INTEL_INFO(dev_priv)->display.has_ipc) #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 2df2db0a5d70..ce6ae6a3cbdf 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1028,6 +1028,7 @@ static const struct intel_device_info xehpsdv_info = { PLATFORM(INTEL_XEHPSDV), .display = { }, .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | @@ -1046,6 +1047,7 @@ static const struct intel_device_info dg2_info = { PLATFORM(INTEL_DG2), .has_guc_deprivilege = 1, .has_64k_pages = 1, + .needs_compact_pt = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index abf1e103c558..d8da40d01dca 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -130,6 +130,7 @@ enum intel_ppgtt_type { /* Keep has_* in alphabetical order */ \ func(has_64bit_reloc); \ func(has_64k_pages); \ + func(needs_compact_pt); \ func(gpu_reset_clobbers_display); \ func(has_reset_engine); \ func(has_global_mocs); \ -- 2.25.1
[PATCH v6 2/5] drm/i915: enforce min GTT alignment for discrete cards
From: Matthew Auld For local-memory objects we need to align the GTT addresses to 64K, both for the ppgtt and ggtt. We need to support vm->min_alignment > 4K, depending on the vm itself and the type of object we are inserting. With this in mind update the GTT selftests to take this into account. For compact-pt we further align and pad lmem object GTT addresses to 2MB to ensure PDEs contain consistent page sizes as required by the HW. v3: * use needs_compact_pt flag to discriminate between 64K and 64K with compact-pt * add i915_vm_obj_min_alignment * use i915_vm_obj_min_alignment to round up vma reservation if compact-pt instead of hard coding v5: * fix i915_vm_obj_min_alignment for internal objects which have no memory region v6: * tiled_blits_create correctly pick largest required alignment Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström Cc: Joonas Lahtinen Cc: Rodrigo Vivi --- .../i915/gem/selftests/i915_gem_client_blt.c | 21 ++-- drivers/gpu/drm/i915/gt/intel_gtt.c | 12 +++ drivers/gpu/drm/i915/gt/intel_gtt.h | 18 drivers/gpu/drm/i915/i915_vma.c | 9 ++ drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 96 --- 5 files changed, 115 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c index c8ff8bf0986d..3675d12a7d9a 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c @@ -39,6 +39,7 @@ struct tiled_blits { struct blit_buffer scratch; struct i915_vma *batch; u64 hole; + u64 align; u32 width; u32 height; }; @@ -410,14 +411,19 @@ tiled_blits_create(struct intel_engine_cs *engine, struct rnd_state *prng) goto err_free; } - hole_size = 2 * PAGE_ALIGN(WIDTH * HEIGHT * 4); + t->align = i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_LOCAL); + t->align = max(t->align, + i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_SYSTEM)); + + hole_size = 2 * round_up(WIDTH * HEIGHT * 4, t->align); hole_size *= 2; /* room to maneuver */ - hole_size += 2 * I915_GTT_MIN_ALIGNMENT; + hole_size += 2 * t->align; /* padding on either side */ mutex_lock(&t->ce->vm->mutex); memset(&hole, 0, sizeof(hole)); err = drm_mm_insert_node_in_range(&t->ce->vm->mm, &hole, - hole_size, 0, I915_COLOR_UNEVICTABLE, + hole_size, t->align, + I915_COLOR_UNEVICTABLE, 0, U64_MAX, DRM_MM_INSERT_BEST); if (!err) @@ -428,7 +434,7 @@ tiled_blits_create(struct intel_engine_cs *engine, struct rnd_state *prng) goto err_put; } - t->hole = hole.start + I915_GTT_MIN_ALIGNMENT; + t->hole = hole.start + t->align; pr_info("Using hole at %llx\n", t->hole); err = tiled_blits_create_buffers(t, WIDTH, HEIGHT, prng); @@ -455,7 +461,7 @@ static void tiled_blits_destroy(struct tiled_blits *t) static int tiled_blits_prepare(struct tiled_blits *t, struct rnd_state *prng) { - u64 offset = PAGE_ALIGN(t->width * t->height * 4); + u64 offset = round_up(t->width * t->height * 4, t->align); u32 *map; int err; int i; @@ -486,8 +492,7 @@ static int tiled_blits_prepare(struct tiled_blits *t, static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng) { - u64 offset = - round_up(t->width * t->height * 4, 2 * I915_GTT_MIN_ALIGNMENT); + u64 offset = round_up(t->width * t->height * 4, 2 * t->align); int err; /* We want to check position invariant tiling across GTT eviction */ @@ -500,7 +505,7 @@ static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng) /* Reposition so that we overlap the old addresses, and slightly off */ err = tiled_blit(t, -&t->buffers[2], t->hole + I915_GTT_MIN_ALIGNMENT, +&t->buffers[2], t->hole + t->align, &t->buffers[1], t->hole + 3 * offset / 2); if (err) return err; diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 46be4197b93f..df23ebdfc994 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -223,6 +223,18 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass) GEM_BUG_ON(!vm->total); drm_mm_init(&vm->mm, 0, vm->total); + + memset64(vm->min_alignment, I91
[PATCH v6 5/5] drm/i915/uapi: document behaviour for DG2 64K support
From: Matthew Auld On discrete platforms like DG2, we need to support a minimum page size of 64K when dealing with device local-memory. This is quite tricky for various reasons, so try to document the new implicit uapi for this. v3: fix typos and less emphasis v2: Fixed suggestions on formatting [Daniel] Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Acked-by: Jordan Justen Reviewed-by: Ramalingam C Reviewed-by: Thomas Hellström cc: Simon Ser cc: Pekka Paalanen Cc: Jordan Justen Cc: Kenneth Graunke Cc: mesa-...@lists.freedesktop.org Cc: Tony Ye Cc: Slawomir Milczarek --- include/uapi/drm/i915_drm.h | 44 - 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 5e678917da70..77e5e74c32c1 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 { /** * When the EXEC_OBJECT_PINNED flag is specified this is populated by * the user with the GTT offset at which this object will be pinned. +* * When the I915_EXEC_NO_RELOC flag is specified this must contain the * presumed_offset of the object. +* * During execbuffer2 the kernel populates it with the value of the * current GTT offset of the object, for future presumed_offset writes. +* +* See struct drm_i915_gem_create_ext for the rules when dealing with +* alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with +* minimum page sizes, like DG2. */ __u64 offset; @@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext { * * The (page-aligned) allocated size for the object will be returned. * -* Note that for some devices we have might have further minimum -* page-size restrictions(larger than 4K), like for device local-memory. -* However in general the final size here should always reflect any -* rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS -* extension to place the object in device local-memory. +* +* DG2 64K min page size implications: +* +* On discrete platforms, starting from DG2, we have to contend with GTT +* page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE +* objects. Specifically the hardware only supports 64K or larger GTT +* page sizes for such memory. The kernel will already ensure that all +* I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page +* sizes underneath. +* +* Note that the returned size here will always reflect any required +* rounding up done by the kernel, i.e 4K will now become 64K on devices +* such as DG2. +* +* Special DG2 GTT address alignment requirement: +* +* The GTT alignment will also need to be at least 2M for such objects. +* +* Note that due to how the hardware implements 64K GTT page support, we +* have some further complications: +* +* 1) The entire PDE (which covers a 2MB virtual address range), must +* contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same +* PDE is forbidden by the hardware. +* +* 2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM +* objects. +* +* To keep things simple for userland, we mandate that any GTT mappings +* must be aligned to and rounded up to 2MB. As this only wastes virtual +* address space and avoids userland having to copy any needlessly +* complicated PDE sharing scheme (coloring) and only affects DG2, this +* is deemed to be a good compromise. */ __u64 size; /** -- 2.25.1
[PATCH v6 3/5] drm/i915: support 64K GTT pages for discrete cards
From: Matthew Auld discrete cards optimise 64K GTT pages for local-memory, since everything should be allocated at 64K granularity. We say goodbye to sparse entries, and instead get a compact 256B page-table for 64K pages, which should be more cache friendly. 4K pages for local-memory are no longer supported by the HW. v4: don't return uninitialized err in igt_ppgtt_compact Reported-by: kernel test robot Signed-off-by: Matthew Auld Signed-off-by: Stuart Summers Signed-off-by: Ramalingam C Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström Cc: Joonas Lahtinen Cc: Rodrigo Vivi --- .../gpu/drm/i915/gem/selftests/huge_pages.c | 60 ++ drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 108 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 3 + drivers/gpu/drm/i915/gt/intel_ppgtt.c | 1 + 4 files changed, 169 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index f36191ebf964..a7d9bdb85d70 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -1478,6 +1478,65 @@ static int igt_ppgtt_sanity_check(void *arg) return err; } +static int igt_ppgtt_compact(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct drm_i915_gem_object *obj; + int err; + + /* +* Simple test to catch issues with compact 64K pages -- since the pt is +* compacted to 256B that gives us 32 entries per pt, however since the +* backing page for the pt is 4K, any extra entries we might incorrectly +* write out should be ignored by the HW. If ever hit such a case this +* test should catch it since some of our writes would land in scratch. +*/ + + if (!HAS_64K_PAGES(i915)) { + pr_info("device lacks compact 64K page support, skipping\n"); + return 0; + } + + if (!HAS_LMEM(i915)) { + pr_info("device lacks LMEM support, skipping\n"); + return 0; + } + + /* We want the range to cover multiple page-table boundaries. */ + obj = i915_gem_object_create_lmem(i915, SZ_4M, 0); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + err = i915_gem_object_pin_pages_unlocked(obj); + if (err) + goto out_put; + + if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_64K) { + pr_info("LMEM compact unable to allocate huge-page(s)\n"); + goto out_unpin; + } + + /* +* Disable 2M GTT pages by forcing the page-size to 64K for the GTT +* insertion. +*/ + obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE_64K; + + err = igt_write_huge(i915, obj); + if (err) + pr_err("LMEM compact write-huge failed\n"); + +out_unpin: + i915_gem_object_unpin_pages(obj); +out_put: + i915_gem_object_put(obj); + + if (err == -ENOMEM) + err = 0; + + return err; +} + static int igt_tmpfs_fallback(void *arg) { struct drm_i915_private *i915 = arg; @@ -1735,6 +1794,7 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915) SUBTEST(igt_tmpfs_fallback), SUBTEST(igt_ppgtt_smoke_huge), SUBTEST(igt_ppgtt_sanity_check), + SUBTEST(igt_ppgtt_compact), }; if (!HAS_PPGTT(i915)) { diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index c43e724afa9f..62471730266c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -233,6 +233,8 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * const vm, start, end, lvl); } else { unsigned int count; + unsigned int pte = gen8_pd_index(start, 0); + unsigned int num_ptes; u64 *vaddr; count = gen8_pt_count(start, end); @@ -242,10 +244,18 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * const vm, atomic_read(&pt->used)); GEM_BUG_ON(!count || count >= atomic_read(&pt->used)); + num_ptes = count; + if (pt->is_compact) { + GEM_BUG_ON(num_ptes % 16); + GEM_BUG_ON(pte % 16); + num_ptes /= 16; + pte /= 16; + } + vaddr = px_vaddr(pt); - memset64(vaddr + gen8_pd_index(start, 0), + memset64(vaddr + pte, vm->scratch[0]->encode, -count); +num_ptes);
[PATCH v6 4/5] drm/i915: add gtt misalignment test
add test to check handling of misaligned offsets and sizes v4: * remove spurious blank lines * explicitly cast intel_region_id to intel_memory_type in misaligned_pin Reported-by: kernel test robot v6: * use NEEDS_COMPACT_PT instead of hard coding for DG2 Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström --- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 128 ++ 1 file changed, 128 insertions(+) diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index b80788a2b7f9..c23b1e5cc436 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -22,10 +22,12 @@ * */ +#include "gt/intel_gtt.h" #include #include #include "gem/i915_gem_context.h" +#include "gem/i915_gem_region.h" #include "gem/selftests/mock_context.h" #include "gt/intel_context.h" #include "gt/intel_gpu_commands.h" @@ -1067,6 +1069,120 @@ static int shrink_boom(struct i915_address_space *vm, return err; } +static int misaligned_case(struct i915_address_space *vm, struct intel_memory_region *mr, + u64 addr, u64 size, unsigned long flags) +{ + struct drm_i915_gem_object *obj; + struct i915_vma *vma; + int err = 0; + u64 expected_vma_size, expected_node_size; + + obj = i915_gem_object_create_region(mr, size, 0, 0); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + vma = i915_vma_instance(obj, vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto err_put; + } + + err = i915_vma_pin(vma, 0, 0, addr | flags); + if (err) + goto err_put; + i915_vma_unpin(vma); + + if (!drm_mm_node_allocated(&vma->node)) { + err = -EINVAL; + goto err_put; + } + + if (i915_vma_misplaced(vma, 0, 0, addr | flags)) { + err = -EINVAL; + goto err_put; + } + + expected_vma_size = round_up(size, 1 << (ffs(vma->resource->page_sizes_gtt) - 1)); + expected_node_size = expected_vma_size; + + if (NEEDS_COMPACT_PT(vm->i915) && i915_gem_object_is_lmem(obj)) { + /* compact-pt should expand lmem node to 2MB */ + expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K); + expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M); + } + + if (vma->size != expected_vma_size || vma->node.size != expected_node_size) { + err = i915_vma_unbind(vma); + err = -EBADSLT; + goto err_put; + } + + err = i915_vma_unbind(vma); + if (err) + goto err_put; + + GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); + +err_put: + i915_gem_object_put(obj); + cleanup_freed_objects(vm->i915); + return err; +} + +static int misaligned_pin(struct i915_address_space *vm, + u64 hole_start, u64 hole_end, + unsigned long end_time) +{ + struct intel_memory_region *mr; + enum intel_region_id id; + unsigned long flags = PIN_OFFSET_FIXED | PIN_USER; + int err = 0; + u64 hole_size = hole_end - hole_start; + + if (i915_is_ggtt(vm)) + flags |= PIN_GLOBAL; + + for_each_memory_region(mr, vm->i915, id) { + u64 min_alignment = i915_vm_min_alignment(vm, (enum intel_memory_type)id); + u64 size = min_alignment; + u64 addr = round_up(hole_start + (hole_size / 2), min_alignment); + + /* we can't test < 4k alignment due to flags being encoded in lower bits */ + if (min_alignment != I915_GTT_PAGE_SIZE_4K) { + err = misaligned_case(vm, mr, addr + (min_alignment / 2), size, flags); + /* misaligned should error with -EINVAL*/ + if (!err) + err = -EBADSLT; + if (err != -EINVAL) + return err; + } + + /* test for vma->size expansion to min page size */ + err = misaligned_case(vm, mr, addr, PAGE_SIZE, flags); + if (min_alignment > hole_size) { + if (!err) + err = -EBADSLT; + else if (err == -ENOSPC) + err = 0; + } + if (err) + return err; + + /* test for intermediate size not expanding vma->size for large alignments */ + err = misaligned_case(vm, mr, addr, size / 2, flags); + if (min_alignment > hole_size) { + if (!err) + err = -EBADSLT; + else if (err == -ENOSPC) + err = 0; +
[PATCH v6 0/5] discrete card 64K page support
This series continues support for 64K pages for discrete cards. It supersedes the 64K patches from https://patchwork.freedesktop.org/series/95686/#rev4 Changes since that series: - set min alignment for DG2 to 2MB in i915_address_space_init - replace coloring with simpler 2MB VA alignment for lmem buffers - enforce alignment to 2MB for lmem objects on DG2 in i915_vma_insert - expand vma reservation to round up to 2MB on DG2 in i915_vma_insert - add alignment test v2: rebase and fix for async vma that landed v3: * fix uapi doc typos * add needs_compact_pt flag patch * cleanup vma expansion to use vm->min_alignment instead of hard coding v4: * fix err return in igt_ppgtt_compact test * placate ci robot with explicit enum conversion in misaligned_pin * remove some blank lines v5: * fix obj alignment requirements querying for internal buffers that have no memory region associated. (fixes v3 bug) v6: * use NEEDS_COMPACT_PT inead of hard coding in misalignment test * tiled_blits_create correctly pick largest required alignment * minor doc formatting Reviewed-by: Thomas Hellström Matthew Auld (3): drm/i915: enforce min GTT alignment for discrete cards drm/i915: support 64K GTT pages for discrete cards drm/i915/uapi: document behaviour for DG2 64K support Ramalingam C (1): drm/i915: add needs_compact_pt flag Robert Beckett (1): drm/i915: add gtt misalignment test .../gpu/drm/i915/gem/selftests/huge_pages.c | 60 + .../i915/gem/selftests/i915_gem_client_blt.c | 21 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 108 - drivers/gpu/drm/i915/gt/intel_gtt.c | 12 + drivers/gpu/drm/i915/gt/intel_gtt.h | 21 ++ drivers/gpu/drm/i915/gt/intel_ppgtt.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 11 +- drivers/gpu/drm/i915/i915_pci.c | 2 + drivers/gpu/drm/i915/i915_vma.c | 9 + drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 224 +++--- include/uapi/drm/i915_drm.h | 44 +++- 12 files changed, 462 insertions(+), 52 deletions(-) -- 2.25.1
Re: [PATCH v11] drm/bridge: add it6505 driver
On Mon, 31 Jan 2022 at 17:55, Hsin-Yi Wang wrote: > > On Tue, Feb 1, 2022 at 12:37 AM Robert Foss wrote: > > > > On Thu, 20 Jan 2022 at 16:25, AngeloGioacchino Del Regno > > wrote: > > > > > > Il 14/01/22 10:14, allen ha scritto: > > > > This adds support for the iTE IT6505. > > > > This device can convert DPI signal to DP output. > > > > > > > > From: Allen Chen > > > > Tested-by: Hsin-yi Wang > > > > Signed-off-by: Hermes Wu > > > > Signed-off-by: Allen Chen > > > > --- > > > > v10 -> v11 : remove drm_bridge_new_crtc_state > > > > --- > > > > drivers/gpu/drm/bridge/Kconfig |8 + > > > > drivers/gpu/drm/bridge/Makefile |1 + > > > > drivers/gpu/drm/bridge/ite-it6505.c | 3352 +++ > > > > 3 files changed, 3361 insertions(+) > > > > create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c > > > > > > > > > > ...snip... > > > > > > > +static const struct of_device_id it6505_of_match[] = { > > > > + { .compatible = "ite,it6505" }, > > > > + { } > > > > +}; > > > > > > If you want to have a DT compatible and DT properties, you have to also > > > add > > > dt-bindings (yaml) for this driver, otherwise, any SoC/device DT will fail > > > the dt binding check So, please, add that. > > > > Let me second this. A dt-binding is needed for this driver to be > > complete, it functions as both documentation and a way to test the DTS > > that use this device, so it is really important. > > > The binding seems to be accepted before the driver: > https://elixir.bootlin.com/linux/v5.16.4/source/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml I completely missed that. In that case we're only missing the reviewed-by tag from someone. > > > > > > > For the driver by itself, though: > > > > > > Acked-by: AngeloGioacchino Del Regno > > > > > > > > > > + > > > > +static struct i2c_driver it6505_i2c_driver = { > > > > + .driver = { > > > > + .name = "it6505", > > > > + .of_match_table = it6505_of_match, > > > > + .pm = &it6505_bridge_pm_ops, > > > > + }, > > > > + .probe = it6505_i2c_probe, > > > > + .remove = it6505_i2c_remove, > > > > + .shutdown = it6505_shutdown, > > > > + .id_table = it6505_id, > > > > +}; > > > > + > > > > +module_i2c_driver(it6505_i2c_driver); > > > > + > > > > +MODULE_AUTHOR("Allen Chen "); > > > > +MODULE_DESCRIPTION("IT6505 DisplayPort Transmitter driver"); > > > > +MODULE_LICENSE("GPL v2"); > > > > > > >
Re: [PATCH v5 0/4] arm: dts: qcom: sc7280: Add display DT nodes for sc7280
On Fri, 24 Dec 2021 21:33:09 +0530, Sankeerth Billakanti wrote: > Add display devicetree support for sc7280 platform. > > Krishna Manikandan (1): > arm64: dts: qcom: sc7280: add display dt nodes > > Kuogee Hsieh (1): > arm64: dts: qcom: sc7280: Add Display Port node > > [...] Applied, thanks! [1/4] arm64: dts: qcom: sc7280: add display dt nodes commit: fcb68dfda5cbd816d27ac50c287833848874f61c [2/4] arm64: dts: qcom: sc7280: Add DSI display nodes commit: 43137272f0bc5e05e4c4c6f7bfce017bfb9e16b5 [3/4] arm64: dts: qcom: sc7280: add edp display dt nodes commit: 25940788d170251373d8975d359706350818fa0f [4/4] arm64: dts: qcom: sc7280: Add Display Port node commit: fc6b1225d20de0298a7b0e52eb3843d71e1992e8 Best regards, -- Bjorn Andersson
Re: [PATCH v4 4/4] arm64: dts: qcom: sc7280: Add Display Port node
On Mon, 22 Nov 2021 16:59:15 +0530, Sankeerth Billakanti wrote: > From: Kuogee Hsieh > > Applied, thanks! [4/4] arm64: dts: qcom: sc7280: Add Display Port node commit: fc6b1225d20de0298a7b0e52eb3843d71e1992e8 Best regards, -- Bjorn Andersson
Re: [PATCH v4 1/4] arm64: dts: qcom: sc7280: add display dt nodes
On Mon, 22 Nov 2021 16:56:06 +0530, Sankeerth Billakanti wrote: > From: Krishna Manikandan > > Add mdss and mdp DT nodes for sc7280. > > Applied, thanks! [1/4] arm64: dts: qcom: sc7280: add display dt nodes commit: fcb68dfda5cbd816d27ac50c287833848874f61c [2/4] arm64: dts: qcom: sc7280: Add DSI display nodes commit: 43137272f0bc5e05e4c4c6f7bfce017bfb9e16b5 [3/4] arm64: dts: qcom: sc7280: add edp display dt nodes commit: 25940788d170251373d8975d359706350818fa0f Best regards, -- Bjorn Andersson
[PATCH v4 2/3] dt-bindings: display: simple: Add HannStar HSD101PWW2
From: Svyatoslav Ryhel Add HannStar HSD101PWW2 10.1" WXGA (1280x800) TFT-LCD LVDS panel to the list of compatibles. Acked-by: Rob Herring Signed-off-by: Svyatoslav Ryhel Signed-off-by: Dmitry Osipenko --- .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 62f5f050c1bc..fe49c4df65fa 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -156,6 +156,8 @@ properties: - hannstar,hsd070pww1 # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel - hannstar,hsd100pxn1 +# HannStar Display Corp. HSD101PWW2 10.1" WXGA (1280x800) LVDS panel + - hannstar,hsd101pww2 # Hitachi Ltd. Corporation 9" WVGA (800x480) TFT LCD panel - hit,tx23d38vm0caa # InfoVision Optoelectronics M133NWF4 R0 13.3" FHD (1920x1080) TFT LCD panel -- 2.34.1
[PATCH v4 1/3] dt-bindings: sharp, lq101r1sx01: Add compatible for LQ101R1SX03
From: Anton Bambura LQ101R1SX03 is compatible with LQ101R1SX01 from software perspective, document it. The LQ101R1SX03 is a newer revision of LQ101R1SX01, it has minor differences in hardware pins in comparison to the older version. The newer version of the panel can be found on Android tablets, like ASUS TF701T. Reviewed-by: Rob Herring Signed-off-by: Anton Bambura Signed-off-by: Dmitry Osipenko --- .../bindings/display/panel/sharp,lq101r1sx01.yaml | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/panel/sharp,lq101r1sx01.yaml b/Documentation/devicetree/bindings/display/panel/sharp,lq101r1sx01.yaml index a679d3647dbd..9ec0e8aae4c6 100644 --- a/Documentation/devicetree/bindings/display/panel/sharp,lq101r1sx01.yaml +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq101r1sx01.yaml @@ -30,7 +30,12 @@ allOf: properties: compatible: -const: sharp,lq101r1sx01 +oneOf: + - items: + - const: sharp,lq101r1sx03 + - const: sharp,lq101r1sx01 + - items: + - const: sharp,lq101r1sx01 reg: true power-supply: true -- 2.34.1
[PATCH v4 3/3] drm/panel: simple: Add support for HannStar HSD101PWW2 panel
From: Svyatoslav Ryhel Add definition of the HannStar HSD101PWW2 Rev0-A00/A01 LCD SuperIPS+ HD panel. Signed-off-by: Svyatoslav Ryhel Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/panel/panel-simple.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 9e46db5e359c..1bfa2d1b61fd 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -1927,6 +1927,31 @@ static const struct panel_desc hannstar_hsd100pxn1 = { .connector_type = DRM_MODE_CONNECTOR_LVDS, }; +static const struct display_timing hannstar_hsd101pww2_timing = { + .pixelclock = { 6430, 7110, 8200 }, + .hactive = { 1280, 1280, 1280 }, + .hfront_porch = { 1, 1, 10 }, + .hback_porch = { 1, 1, 10 }, + .hsync_len = { 58, 158, 661 }, + .vactive = { 800, 800, 800 }, + .vfront_porch = { 1, 1, 10 }, + .vback_porch = { 1, 1, 10 }, + .vsync_len = { 1, 21, 203 }, + .flags = DISPLAY_FLAGS_DE_HIGH, +}; + +static const struct panel_desc hannstar_hsd101pww2 = { + .timings = &hannstar_hsd101pww2_timing, + .num_timings = 1, + .bpc = 8, + .size = { + .width = 217, + .height = 136, + }, + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, + .connector_type = DRM_MODE_CONNECTOR_LVDS, +}; + static const struct drm_display_mode hitachi_tx23d38vm0caa_mode = { .clock = 3, .hdisplay = 800, @@ -3802,6 +3827,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "hannstar,hsd100pxn1", .data = &hannstar_hsd100pxn1, + }, { + .compatible = "hannstar,hsd101pww2", + .data = &hannstar_hsd101pww2, }, { .compatible = "hit,tx23d38vm0caa", .data = &hitachi_tx23d38vm0caa -- 2.34.1